[PATCH] D69411: [MC] Calculate difference of symbols in two fragments when possible

Tue Nov 5 16:27:40 PST 2019

jcai19 added a comment.

In D69411#1732215 <https://reviews.llvm.org/D69411#1732215>, @peter.smith wrote:

> I think that this needs some wider review for a couple of reasons. It looks like this is heading towards a special case evaluation just for .if. I'm not comfortable going too much further in the approval process as this is exceeding my knowledge of MC, and I'd like see some supporting opinions on whether this is the right thing to do. Will be worth adding some more reviewers, particularly someone familiar with Mach-O. You may need an RFC to the llvm-dev mailing list to draw some attention.

I have sent an RFC and am waiting for comments.

> I'd also like to see if there is a way of implementing this without special casing .if. That is a pragmatic solution for a specific problem, but it does make the code harder to understand so it has a cost. Looking at the reloc-diff.s test case the commit log says:
> 
>   MC/Mach-O: Use the SECTDIFF relocation type for (A - B + constant) where A is external.
>    - I'm not sure why, but this is what 'as' does.
> 
> 
> I've not been able to easily find a definition of what SECTDIFF is, but it appears to be a relocation that supports subtraction, I don't think that there is an equivalent in ELF. It maybe that the case can be resolved by not doing the folding for MachO.
> 
> some other specific comments.
> 
> - Some of the examples I gave would only fail in Thumb as Arm instructions are all the same size, whereas some Thumb-2 instructions are assembled initially as the 2-byte narrow form, and are relaxed to the wider 4-byte form if the instruction is out of range. You'll need something like -triple=thumbv7a-linux-gnueabihf to see the errors. I'd not expect some of these to fail on GNU as (that is a 2-pass assembler).

Thanks for the clarification. I have included all the four test cases.

> - If you can make an X86_64 test I recommend doing so as few contributors will have this optionally removed. Substantially more have ARM optionally removed, as do several buildbots.

Yes, I agree. Will try to make test cases for x64.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69411/new/

https://reviews.llvm.org/D69411