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

[James Y Knight via Phabricator via llvm-commits]

jyknight added a comment.

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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69411