[PATCH] D69411: [MC] Parse .if conditions with symbols in consecutive MCDataFragements

Tue Nov 12 14:03:11 PST 2019

jcai19 added a comment.

In D69411#1742896 <https://reviews.llvm.org/D69411#1742896>, @jyknight wrote:

> Making this work only on ".if" is IMO a non-starter, at least without understanding why. So I looked into what broke with llvm/test/MC/MachO/reloc-diff.s.
>
> Basically, with LLVM's current representation, assuming that consecutive fragments are never moved with respect to each-other is invalid. In both ELF and Mach-O. In ELF, we have the ability to use numbered subsections that can insert new fragments between existing fragments. In Mach-O, fragments can effectively be turned into subsections with the 'subsections_via_symbols' directive. Committing this patch as-is would both be ugly and wrong, since we'd allow computing nonsense offsets -- even if we only restrict it to ".if".
>
> I think it's a mistake that it works like that, and I'm going to spend a little bit of time to see if it'll be easy to fix this representational mistake in LLVM (in a separate patch), so that we have fragment lists which _are_ guaranteed to be kept exactly in the order they appear.
>
> For this review, I have two requests:
>
> 1. Undo all the ".if"-only hacks. (understanding that it will cause some tests to fail, for now).
> 2. Fix the code to support arbitrary numbers of fragments between the symbols, of kinds MCFillFragment and MCDataFragment (and maybe others if they are fixed size -- I haven't looked through all of the kinds). Probably best would be to introduce a helper function to calculate the delta between two arbitrary symbols, given an optional layout (and either return an answer or a failure indication). This should make AttemptToFoldSymbolOffsetDifference significantly more straightforward.
>
>   I believe these examples should work, and will after making the latter change: ``` 1: .arch armv7a nop .arch armv4 nop .arch armv7a nop .if . - 1b != 0 .word 0x12345678 .endif
>
>   2: nop .zero 0x10000 nop .if . - 2b == 4 .word 0x12345678 .endif ```

Thanks for the clarification. It will be great if we can remove the restriction to ".if". I will making changes accordingly while the representation of subsections are being changed.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69411