[PATCH] D153096: [MC] Fold A-B when A's fragment precedes B's fragment

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added a comment.

                



================
Comment at: llvm/test/MC/ARM/directive-if-subtraction.s:23
+// OBJ-NOT:[[@LINE-1]]:5: error: expected absolute expression
+// ASM:[[@LINE-2]]:5: error: expected absolute expression
+// DISASM: orr	r1, r1, #2
----------------
nickdesaulniers wrote:
> MaskRay wrote:
> > nickdesaulniers wrote:
> > > I thought the whole point of this patch is to allow `9997b - . == 0` to fold? Then why is this testing for that to emit an error?
> > This test complements the previous test that we still have `// ASM:[[@LINE-2]]:5: error: expected absolute expression`.
> > 
> > This is related to a larger issue that for now we use `MCAsmStreamer::getAssemblerPtr` that returns null (related to D45164). I have found that improving this would be challenging.
> I see; the run lines are super perplexing to me; changing the output type from obj (to whatever the implicit default is, another .s file perhaps?) causes the assembler to error? WEIRD
Ah, this patch adds a label difference test that serves as a dual purpose:

* -filetype=obj now folds `if 9997b - . == 0`
* `-filetype=asm` cannot fold such expressions

The current summary

> Add a case to llvm/test/MC/ARM/directive-if-subtraction.s that we don't evaluate .if . - 9997b == 0 for MCAsmStreamer due to getAssemblerPtr returns nullptr.


================
Comment at: llvm/test/MC/MachO/reloc-diff.s:4
 _local_def:
+.p2align 2
         .globl _external_def
----------------
nickdesaulniers wrote:
> nickdesaulniers wrote:
> > indent
> This was marked done but does not look fixed to me, please triple check.  Later directives are indented. Please indent this one to be consistent.
Sorry, missed this one. I'll indent it.

(I considered `.p2align 2` similar to the function label, as some directives guarding the following body, so I did not indent it. But I can see an argument for just indenting every align directives.

I think this might be a minor issue of the traditional compiler-generated directives. For example, not indenting `.globl` probably would improve readability.
)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153096