[SystemZ] WRONG code with packed struct with bitfield members.

[JonPsson1]

This program uses struct bitfields, where the first one is 4 bytes of padding, and should print 2:

int printf(const char *, ...);
struct S0 {
  signed : 32;
  signed f2 : 19;
  unsigned f3 : 18;
} S;

volatile struct S0 *P = &S;

long LVar;

static void func_5(struct S0 Arg) {
  LVar = Arg.f2;
}

int main() {
  struct S0 L = {1, 2};
  func_5(L);
  *P = L;
  printf("checksum = %X\n", S.f3);
}

With packed structs it however prints 0:

clang -O3 -march=z16 wrong_dagcombiner.i -o a.out -fno-inline -o ./a.out; ./a.out
checksum = 2

clang -O3 -march=z16 wrong_dagcombiner.i -o a.out -fno-inline -fpack-struct -o ./a.out; ./a.out
checksum = 0

Before isel:
  tail call fastcc void @func_5(i72 2097168)

# *** IR Dump After SystemZ DAG->DAG Pattern Instruction Selection (systemz-isel) ***:
# Machine code for function main: IsSSA, TracksLiveness
Frame Objects:
  fi#0: size=1, align=2, at location [SP]
  fi#1: size=1, align=2, at location [SP]
  fi#2: size=1, align=2, at location [SP]
  fi#3: size=1, align=2, at location [SP]
  fi#4: size=1, align=2, at location [SP]
  fi#5: size=1, align=2, at location [SP]
  fi#6: size=1, align=2, at location [SP]
  fi#7: size=1, align=2, at location [SP]
  fi#8: size=1, align=2, at location [SP]
  fi#9: size=9, align=8, at location [SP]                 // SIZE 9
Constant Pool:
  cp#0: 2097168, align=8

bb.0.entry:
  LIFETIME_START %stack.0.L.sroa.0
  LIFETIME_START %stack.1.L.sroa.5
  LIFETIME_START %stack.2.L.sroa.6
  LIFETIME_START %stack.3.L.sroa.7
  LIFETIME_START %stack.4.L.sroa.8
  LIFETIME_START %stack.5.L.sroa.9
  LIFETIME_START %stack.6.L.sroa.10
  LIFETIME_START %stack.7.L.sroa.11
  LIFETIME_START %stack.8.L.sroa.12
  MVI %stack.4.L.sroa.8, 0, 0 :: (store (s8) into %ir.L.sroa.8, align 2)
  MVI %stack.5.L.sroa.9, 0, 0 :: (store (s8) into %ir.L.sroa.9, align 2)
  MVI %stack.6.L.sroa.10, 0, 32 :: (store (s8) into %ir.L.sroa.10, align 2)
  MVI %stack.7.L.sroa.11, 0, 0 :: (store (s8) into %ir.L.sroa.11, align 2)
  MVI %stack.8.L.sroa.12, 0, 16 :: (store (s8) into %ir.L.sroa.12, align 2)
  ADJCALLSTACKDOWN 0, 0
  %0:addr64bit = LARL %const.0
  %1:vr128bit = VL killed %0:addr64bit, 0, $noreg :: (load (s128) from constant-pool, align 8)
  VST killed %1:vr128bit, %stack.9, 0, $noreg :: (store (s128) into %stack.9, align 8)              // STORE-SIZE 16!

The argument to @func_5 has a size of 9, but 16 bytes are stored, which looks wrong. The i72 is rounded up to i128, which seems counterproductive in regards to the packing. In any case the frame object size must match. Should an i72 struct that is packed maybe be split instead of promoted?

        mvi     182(%r15), 0
        mvi     180(%r15), 0
        mvi     178(%r15), 32
        mvi     176(%r15), 0
        mvi     174(%r15), 16
        vst     %v0, 160(%r15), 3
        brasl   %r14, func_5@PLT

In the output, the vst overwrites the mvi...

tc_packedstructbitfields.tar.gz
llc -mtriple=s390x-linux-gnu -mcpu=z16 -O3 tc_packedstructbitfields.ll

@uweigand

[JonPsson1]

I think this is related SystemZ handling of i128, after all. In SystemZTargetLowering::LowerCall() there is a special handling ("// Allocate the full stack space for a promoted (and split) argument."), that checks explicitly for an argument occupying two registers. So with -EC12, that handling takes action as the argument is split, but with vector and legal i128 it does not as there is only one part.

[JonPsson1]

@uweigand

[uweigand]

Hi @JonPsson1 I think this line at https://github.com/llvm/llvm-project/blob/7f3980a7b2c9f95ab3b106a94fe6e63158155b0b/llvm/lib/Target/SystemZ/SystemZISelLowering.cpp#L1926C1-L1926C32:

        SlotVT = Outs[I].ArgVT;

is simply wrong. It should be Outs[I].VT instead of ArgVT. (At this point ArgVT is the original type before any extension, while VT is the type of the argument (part) that is actually being passed.)

Can you try out and test that change? Thanks!