[PATCH] D15335: [ELF] - R_X86_64_SIZE64/R_X86_64_SIZE32 relocations implemented.

George Rimar via llvm-commits llvm-commits at lists.llvm.org
Wed Dec 9 01:48:03 PST 2015


grimar added inline comments.

================
Comment at: ELF/Target.cpp:645
@@ -638,1 +644,3 @@
+    return;
+
   switch (Type) {
----------------
ruiu wrote:
> grimar wrote:
> >   // This check could be moved at upper level, but that
> >   // would require adding Target->isSizeReloc() method.
> >   // SIZE32/64 relocations exist only for x64 now,
> >   // so it does not need such generic handling and that is why it is here.
> I don't understand why you had to do this. Is this described in the spec?
Point was to skip unnecessary calculations below since anyways dynamic relocation should be created for that case. 
But I did a mistake in logic. Not Config->Shared have to be checked but canBePreempted(). If this relocation is against something that CBP then dynamic relocation is created and there is no need to perform any calculations.
Updated the test to cover that case.

Unfortunately I was need to add isSizeDynReloc() to target that wanted to avoid. Another way I see - to add CBP or Body argument for relocateOne and perform check inside like in first version of patch.


http://reviews.llvm.org/D15335





More information about the llvm-commits mailing list