[AIX] Handle arbitrary sized integers when lowering formal arguments passed on the stack

[amy-kwan]

When arbitrary sized (non-simple type, or non-power of two types) integers are passed on the stack, these integers are not handled when lowering formal arguments on AIX as we always assume we will encounter simple type integers.

However, it is possible for frontends to generate arbitrary sized immediate values in IR. Specifically in rustc, it will generate an integer value in LLVM IR for small structures that are less than a pointer size, which is done for optimization purposes for the Rust ABI. For example, if a Rust structure of three characters is passed into function on the stack,

struct my_struct {
  field1: u8,
  field2: u8,
  field3: u8,
}

This will generate an i24 type in LLVM IR.

Currently, it is not obvious for the backend to distinguish an integer versus something that wasn't an integer to begin with (such as a struct), and the latter case would not have an extend on the parameter. Thus, this PR allows us to perform a truncation and extend on integers, both non-simple and simple types.

[llvmbot]

@llvm/pr-subscribers-backend-powerpc

Author: Amy Kwan (amy-kwan)

Changes

When arbitrary sized (non-simple type, or non-power of two types) integers are passed on the stack, these integers are not handled when lowering formal arguments on AIX as we always assume we will encounter simple type integers.

However, it is possible for frontends to generate arbitrary sized immediate values in IR. Specifically in rustc, it will generate an integer value in LLVM IR for small structures that are less than a pointer size, which is done for optimization purposes for the Rust ABI. For example, if a Rust structure of three characters is passed into function on the stack,

struct my_struct {
  field1: u8,
  field2: u8,
  field3: u8,
}

This will generate an i24 type in LLVM IR.

Currently, it is not obvious for the backend to distinguish an integer versus something that wasn't an integer to begin with (such as a struct), and the latter case would not have an extend on the parameter. Thus, this PR allows us to perform a truncation and extend on integers, both non-simple and simple types.

---

Full diff: https://github.com/llvm/llvm-project/pull/149351.diff

2 Files Affected:

• (modified) llvm/lib/Target/PowerPC/PPCISelLowering.cpp (+12-2)
• (added) llvm/test/CodeGen/PowerPC/aix-lower-arbitrary-sized-ints.ll (+33)

diff --git a/llvm/lib/Target/PowerPC/PPCISelLowering.cpp b/llvm/lib/Target/PowerPC/PPCISelLowering.cpp
index 459525ed4ee9a..13a29b78f0612 100644
--- a/llvm/lib/Target/PowerPC/PPCISelLowering.cpp
+++ b/llvm/lib/Target/PowerPC/PPCISelLowering.cpp
@@ -7296,9 +7296,19 @@ SDValue PPCTargetLowering::LowerFormalArguments_AIX(
       if (!ArgVT.isVector() && !ValVT.isVector() && ArgVT.isInteger() &&
           ValVT.isInteger() &&
           ArgVT.getScalarSizeInBits() < ValVT.getScalarSizeInBits()) {
+        // It is possible to have either real integer values that aren't
+        // the power of two sizes, or integers that were not originally
+        // integers. In the latter case, these could have came from structs,
+        // and these integers would not have an extend on the parameter.
+        // Since these types of integers do not have an extend specified
+        // in the first place, the type of extend that we do should not matter.
+        EVT TruncatedArgVT;
+        if (ArgVT.isSimple())
+          TruncatedArgVT = ArgVT.getSimpleVT() == MVT::i1 ? MVT::i8 : ArgVT;
+        else
+          TruncatedArgVT = ArgVT;
         SDValue ArgValueTrunc = DAG.getNode(
-            ISD::TRUNCATE, dl, ArgVT.getSimpleVT() == MVT::i1 ? MVT::i8 : ArgVT,
-            ArgValue);
+            ISD::TRUNCATE, dl, TruncatedArgVT, ArgValue);
         SDValue ArgValueExt =
             ArgSignExt ? DAG.getSExtOrTrunc(ArgValueTrunc, dl, ValVT)
                        : DAG.getZExtOrTrunc(ArgValueTrunc, dl, ValVT);
diff --git a/llvm/test/CodeGen/PowerPC/aix-lower-arbitrary-sized-ints.ll b/llvm/test/CodeGen/PowerPC/aix-lower-arbitrary-sized-ints.ll
new file mode 100644
index 0000000000000..297d2a3e10f94
--- /dev/null
+++ b/llvm/test/CodeGen/PowerPC/aix-lower-arbitrary-sized-ints.ll
@@ -0,0 +1,33 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 5
+; RUN: llc --verify-machineinstrs -mtriple powerpc-ibm-aix-xcoff \
+; RUN:   -mcpu=pwr8 < %s | FileCheck %s --check-prefixes=CHECK,CHECK32
+; RUN: llc --verify-machineinstrs -mtriple powerpc64-ibm-aix-xcoff \
+; RUN:   -mcpu=pwr8 < %s | FileCheck %s --check-prefixes=CHECK,CHECK64
+
+define ptr @lower_args(ptr %_0, i32 %0, i32 %1, i32 %2, i32 %3, ptr %4, ptr %5, i64 %6, i24 %7) {
+; CHECK-LABEL: lower_args:
+; CHECK:       # %bb.0: # %entry
+; CHECK-NEXT:    blr
+entry:
+  ret ptr %_0
+}
+
+define i32 @lower_args2(i32 %a, i32 %b, i32 %c, i32 %d, i32 %e, i32 %f, i32 %g, i32 %h, i24 %i) {
+; CHECK32-LABEL: lower_args2:
+; CHECK32:       # %bb.0: # %entry
+; CHECK32-NEXT:    lwz 3, 56(1)
+; CHECK32-NEXT:    addi 3, 3, 255
+; CHECK32-NEXT:    clrlwi 3, 3, 8
+; CHECK32-NEXT:    blr
+;
+; CHECK64-LABEL: lower_args2:
+; CHECK64:       # %bb.0: # %entry
+; CHECK64-NEXT:    lwz 3, 116(1)
+; CHECK64-NEXT:    addi 3, 3, 255
+; CHECK64-NEXT:    clrldi 3, 3, 40
+; CHECK64-NEXT:    blr
+entry:
+  %0 = add i24 %i, 255
+  %1 = zext i24 %0 to i32
+  ret i32 %1
+}

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[amy-kwan]

A couple notes and observations:

• https://github.com/rust-lang/rust/blob/master/compiler/rustc_target/src/callconv/mod.rs#L800 is where the rust frontend generates an immediate for small structures less than a pointer size. It seems than any structures that are larger than a pointer size will generate ptr instead in the IR. This is only done for the Rust ABI. I've taken a look previously, and if we are using the C ABI from Rust, we generate byval for structures instead, which should be expected.
• The NVPTX backend ran into this similar situation when lowering calls. Specifically, they encountered an i24 type that came from the rust frontend, exactly in the case where we have a struct with three characters, where NVPTX: Cannot select: = NVPTXISD::LoadParam<(load (s24), align 4)> #55764 describes this situation in detail. A sign or zero extend appears to be done for this case, regardless if an extend is found on the parameter.
• Non-simple types like i24 without an extend on the function parameter has also been seen on PPC64LE: 354d310

[daltenty]

nit: this could simplify, since the default is to return ArgVT:

Suggested change

	if (ArgVT.isSimple())
	TruncatedArgVT = ArgVT.getSimpleVT() == MVT::i1 ? MVT::i8 : ArgVT;
	else
	TruncatedArgVT = ArgVT;
	TruncatedArgVT = ArgVT.isSimple() && ArgVT.getSimpleVT() == MVT::i1 ? MVT::i8 : ArgVT;

I like writing it this way, because it makes clear that the only real remaining special case is i1's.

[daltenty]

nit: I wonder about the placement of this comment, seems to me it belongs at the line where we choose the sigext, not here.

Also, as we are just doing what we do for everything else now (i.e. there's nothing special about non-power of 2 types), it seems to me the comment could be generalized (you can do the same struct trick with an i64, it's not unique to a simple type).

Suggested change

	// It is possible to have either real integer values that aren't
	// the power of two sizes, or integers that were not originally
	// integers. In the latter case, these could have came from structs,
	// and these integers would not have an extend on the parameter.
	// Since these types of integers do not have an extend specified
	// in the first place, the type of extend that we do should not matter.
	// It is possible to have either real integer values
	// or integers that were not originally integers.
	// In the latter case, these could have came from structs,
	// and these integers would not have an extend on the parameter.
	// Since these types of integers do not have an extend specified
	// in the first place, the type of extend that we do should not matter.

[daltenty]

LGTM, with some minor nits

[daltenty]

nit: I guess you could add the other case with an explicit sign extension.

[amy-kwan]

@hubert-reinterpretcast @mandlebug @RolandF77 Did any of you have any additional comments regarding this patch?