[PATCH] [lld][ELF][ARM] Add veneer generation to branch instructions

Denis Protivensky dprotivensky at accesssoftek.com
Tue Feb 10 01:19:56 PST 2015


================
Comment at: lib/ReaderWriter/ELF/ARM/ARMRelocationPass.cpp:157
@@ +156,3 @@
+    const auto kindValue = ref.kindValue();
+    if (R_ARM_CALL == kindValue || R_ARM_THM_CALL == kindValue)
+      // TODO: need additional check of arch type as it should be
----------------
denis-protivensky wrote:
> wnewton wrote:
> > denis-protivensky wrote:
> > > wnewton wrote:
> > > > 
> > > > It seems like it might be simpler not to pass R_*_CALL relocations into this function at all then we could remove the handling here and in the switch statement below (but retain a comment about ARMv5).
> > > Will, I'm not sure I understand the idea. I take value of relocation from the reference object, and I cannot but pass it into the function as it's used in couple more places.
> > > If I change comparison here and the switch down the code, I'll need to pass couple of flags into the function: isThumbCode, isCall and maybe more.
> > > Could you please explain your thought in more detail?
> > 
> > Here we just return if we find kindValue is R_ARM_CALL or R_ARM_THM_CALL but then we test kindValue again lower down in the switch where these values can now never occur. handleVeneer is only called in one place and in that switch we explicitly call it for R_ARM_CALL and R_ARM_THM_CALL even though handleVeneer seems to be an effective noo-op in those cases.
> Okay, I got what confused you. My intention was to form fully-working skeleton code with all checks needed. Further, when architecture parsing option is added, it'll be simpler to just walk through all TODO records and add the corresponding checks (and the tests, of course).
> Do you find these stubs excessive?
I thought a bit more about what you said, and I agree that these checks for ARM_*_CALL should be removed. At least because they're not covered with tests and do not bring any new functionality yet. I'll leave a TODO mark instead as you proposed. Thanks!

http://reviews.llvm.org/D7502

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the llvm-commits mailing list