[PATCH] D19908: [X86] Support the "ms-hotpatch" attribute.

Charles Davis via llvm-commits llvm-commits at lists.llvm.org
Wed Aug 17 11:46:10 PDT 2016


cdavis5x reopened this revision.
cdavis5x added a comment.
This revision is now accepted and ready to land.

I think I've figured out why the build broke when I landed this. (Sorry that took so long.)

When I added the `emitPatchableOp()` method to `TargetTransformInfo`, I added a dependency of Analysis on CodeGen, as well. But, I didn't record that in the build files. I didn't see any problems because I normally build with `LLVM_LINK_LLVM_DYLIB=ON`, so all of LLVM gets linked into a giant dylib, and everything that uses LLVM gets linked to this giant dylib. The buildbots, on the other hand, don't do this. Since the build system didn't know that Analysis now depended on CodeGen, **BOOM**.

I could've just added the dependency and been done with it, but this got me thinking. CodeGen //already// depends on Analysis. Having Analysis depend on CodeGen would produce a circular dependency. I don't think that's a good thing. I also became convinced that adding `emitPatchableOp()` to `TargetTransformInfo` constituted a layering violation. So, I moved it somewhere else--into `TargetInstrInfo` instead. The base `TargetInstrInfo` is already part of CodeGen, so no new dependencies are added by this change.

I don't want to break the build again like that. I also made an (IMO) not insignificant change to this patch. So, I'd like you all to review this patch again. (Sorry, but this is the last time, I promise!) I've verified that this will not break the build again.


Repository:
  rL LLVM

https://reviews.llvm.org/D19908





More information about the llvm-commits mailing list