[PATCH] D146059: [SystemZ] Allow fp/int casting into inline assembly operands

Tue Mar 21 11:24:33 PDT 2023

uweigand added inline comments.

================
Comment at: llvm/lib/Target/SystemZ/SystemZISelLowering.cpp:1115
       weight = CW_Register;
     break;

----------------
Also, this makes me wonder now if we should return `CW_Invalid` here if soft-float (and similarly for 'v' and no-vector).

================
Comment at: llvm/lib/Target/SystemZ/SystemZISelLowering.cpp:1111
+    else
+      weight = CW_Default;
     break;
----------------
jonpa wrote:
> uweigand wrote:
> > All this `CW_Default` stuff is confusing - isn't this, well, the default?
> Yeah, I was just following the pattern in use, but I guess it's more readable this way.
Ah, sorry - I overlooked the default was `CW_Invalid`, not `CW_Default`.   And in fact the default has to be `CW_Invalid`, since this is critical for the various constant constraints like `'I'`.  If the argument is not an in-range constant, this has to be refused.

================
Comment at: llvm/test/CodeGen/SystemZ/inline-asm-fp-int-casting.ll:38
+; Z15-NEXT:    vl %v0, 0(%r3), 3
+; Z15-NEXT:    vrepg %v2, %v0, 1
+; Z15-NEXT:    #APP
----------------
This difference between z15 and z13 is a bit weird.  Both instruction sequences would be correct and available on both architectures, so I'm not sure why it chooses a different one.  (Also, I'm not actually sure which one performs better ...)   -  That may be something to look at as a follow-on.

================
Comment at: llvm/test/CodeGen/SystemZ/soft-float-inline-asm-01.ll:10

-; CHECK: error: couldn't allocate output register for constraint 'f'
+; CHECK: LLVM ERROR: can't use 'f' constraint with soft-float.
----------------
Hmm, I think the previous error (reported via `emitInlineAsmError`) is better - this is not an LLVM bug (which is what `report_fatal_error` is primarily for), but rather a user error.

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

https://reviews.llvm.org/D146059