[PATCH] D107196: [AArch64InstPrinter] Change printAddSubImm to only add imm value comment when shifted

Tue Aug 3 01:39:05 PDT 2021

jhenderson added a comment.

In D107196#2921591 <https://reviews.llvm.org/D107196#2921591>, @jasonmolenda wrote:

> In D107196#2921590 <https://reviews.llvm.org/D107196#2921590>, @t.p.northover wrote:
>
>> In D107196#2917773 <https://reviews.llvm.org/D107196#2917773>, @MaskRay wrote:
>>
>>> Consider committing the test update separately from the code patch, so that the functional change has very few lines of update.
>>
>> Wouldn't that mean a revision with lots of tests failing? I think a single large commit would be better. People can grep out changes in llvm/test if they're not interested in that.
>
> I figured I would do commits locally and push them at the same time.  I'm doing a final testsuite run right now and was hoping to land it if you think this might be a poor idea.

I wouldn't land them as two separate commits - even though the build bots might be fine, it'll mean that for at least one commit, LLVM as a whole is in a broken state, which can cause various issues e.g. when bisecting, or if a system does a commit-by-commit build and test cycle.

================
Comment at: llvm/test/tools/llvm-objdump/ELF/AArch64/disassemble-align.s:6
 #       CHECK:0000000000000000 <$x.0>:
-#  CHECK-NEXT:       0: 62 10 00 91  |add|x2, x3, #4              // =4
+#  CHECK-NEXT:       0: 62 10 00 91  |add|x2, x3, #4
 #  CHECK-NEXT:       4: 1f 20 03 d5  |nop
----------------
MaskRay wrote:
> Perhaps append `{{$}}` to ensure there is no comment.
+1

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107196