JonPsson1 wrote: I have looked through the changes and made some comments inline.
I built this with expensive checks enabled with all checks passing, and SPEC built successfully. Commenting: ``` @@ -293,7 +293,7 @@ SystemZTargetLowering::SystemZTargetLowering(const TargetMachine &TM, setOperationAction(ISD::ATOMIC_LOAD_UMIN, MVT::i32, Custom); setOperationAction(ISD::ATOMIC_LOAD_UMAX, MVT::i32, Custom); - // Even though i128 is not a legal type, we still need to custom lower + // Even though i128 is not a legal type, we still need to custom lower **// Update comment** @@ -2144,7 +2145,7 @@ CanLowerReturn(CallingConv::ID CallConv, VerifyVectorTypes(Outs); // Special case that we cannot easily detect in RetCC_SystemZ since - // i128 is not a legal type. + // i128 is not a legal type. **// Update comment** +++ b/llvm/lib/Target/SystemZ/SystemZRegisterInfo.td @@ -124,7 +124,7 @@ defm GRX32 : SystemZRegClass<"GRX32", [i32], 32, R12L,R12H,R13L,R13H,R14L,R14H,R15L,R15H) ]>; -// The architecture doesn't really have any i128 support, so model the +// The architecture doesn't really have any i128 support, so model the **// Update comment** ``` I happened to notice some cases with room for improvement: ``` ; Scalar load + insertion + replication could be just a vlrepb. define i128 @fun0(i128 %a, i128 %sh) { ; CHECK-LABEL: fun0: ; CHECK: # %bb.0: ; CHECK-NEXT: l %r0, 12(%r4) // ; CHECK-NEXT: vlvgp %v1, %r0, %r0 // ; CHECK-NEXT: vl %v0, 0(%r3), 3 ; CHECK-NEXT: vrepb %v1, %v1, 15 // ===> vlrepb %v1, 12(%r4) ? ; CHECK-NEXT: vslb %v0, %v0, %v1 ; CHECK-NEXT: vsl %v0, %v0, %v1 ; CHECK-NEXT: vst %v0, 0(%r2), 3 ; CHECK-NEXT: br %r14 %res = shl i128 %a, %sh ret i128 %res } ; %v1 is the shift amount in a VR already. define i128 @fun1(i128 %a, i128 %sh, i128 %t) { ; CHECK-LABEL: fun1: ; CHECK: # %bb.0: ; CHECK-NEXT: vl %v1, 0(%r5), 3 ; CHECK-NEXT: vl %v2, 0(%r4), 3 ; CHECK-NEXT: vaq %v1, %v2, %v1 ; CHECK-NEXT: vlgvf %r0, %v1, 3 // ; CHECK-NEXT: vlvgp %v1, %r0, %r0 // ; CHECK-NEXT: vl %v0, 0(%r3), 3 ; CHECK-NEXT: vrepb %v1, %v1, 15 // ===> vrepb %v1, %v1, 15 ; CHECK-NEXT: vslb %v0, %v0, %v1 ; CHECK-NEXT: vsl %v0, %v0, %v1 ; CHECK-NEXT: vst %v0, 0(%r2), 3 ; CHECK-NEXT: br %r14 %s = add i128 %sh, %t %res = shl i128 %a, %s ret i128 %res } ``` As a side question: I forgot why we can get CCMask '5' here: it seems it should be CCMASK_CMP_NE ('6'), if we reverse the LOC operation..? ``` VTM killed %5:vr128bit, killed %4:vr128bit, implicit-def $cc %6:gr64bit = LOCGR killed %3:gr64bit(tied-def 0), killed %2:gr64bit, 13, 8, implicit killed $cc # *** IR Dump After Two-Address instruction pass (twoaddressinstruction) ***: (SystemZInstrInfo::commuteInstructionImpl) VTM killed %5:vr128bit, killed %4:vr128bit, implicit-def $cc %6:gr64bit = COPY killed %2:gr64bit %6:gr64bit = LOCGR %6:gr64bit(tied-def 0), killed %3:gr64bit, 13, 5, implicit killed $cc ``` https://github.com/llvm/llvm-project/pull/74625 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits