[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