[PATCH] D158473: [AArch64] Check opcode before trying to extract register from operand

Matt Devereau via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Aug 22 03:19:47 PDT 2023


MattDevereau requested changes to this revision.
MattDevereau added a comment.
This revision now requires changes to proceed.

The change looks good to me but the test needs tightening up.

`inline-asm-empty-template.ll` Is too generic of a file name, and it's not actually a template. This should be specifically referencing fneg.



================
Comment at: llvm/test/CodeGen/AArch64/inline-asm-empty-template.ll:6
+; CHECK:       a:                                      // @a
+define void @a(double %c) {
+; CHECK:      // %bb.0:                               // %entry
----------------
This test should be renamed to something that reflects what this is actually testing instead of just `@a`. `@emit_fneg_with_non_register_operand` or something along those lines. At a glance you can't really tell why this test exists out of context with the changes.


================
Comment at: llvm/test/CodeGen/AArch64/inline-asm-empty-template.ll:17
+  %3 = tail call double asm sideeffect "", "=w,0"(double %2)
+  %fneg = fneg double %1
+  %cmp = fcmp oeq double %3, %fneg
----------------
This line should be explicitly checked, or else the test is too flimsy. I can replace this line with `%fneg = fadd double %1, %1` which doesn't test your change at all and this test still passes.

`; CHECK: fneg d1, d0`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158473



More information about the llvm-commits mailing list