[all-commits] [llvm/llvm-project] e4ecd8: [llvm][AArch64] Handle arrays of struct properly (...

[David Spickett via All-commits]

  Branch: refs/heads/main
  Home:   https://github.com/llvm/llvm-project
  Commit: e4ecd83fe969c77c23f7a5bd4d5ed521550b5737
      https://github.com/llvm/llvm-project/commit/e4ecd83fe969c77c23f7a5bd4d5ed521550b5737
  Author: David Spickett <david.spickett at linaro.org>
  Date:   2021-06-16 (Wed, 16 Jun 2021)

  Changed paths:
    M llvm/include/llvm/CodeGen/TargetLowering.h
    M llvm/lib/CodeGen/GlobalISel/CallLowering.cpp
    M llvm/lib/CodeGen/SelectionDAG/FastISel.cpp
    M llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
    M llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
    M llvm/lib/Target/AArch64/AArch64ISelLowering.h
    M llvm/lib/Target/ARM/ARMISelLowering.cpp
    M llvm/lib/Target/ARM/ARMISelLowering.h
    M llvm/lib/Target/PowerPC/PPCISelLowering.h
    A llvm/test/CodeGen/AArch64/argument-blocks-array-of-struct.ll

  Log Message:
  -----------
  [llvm][AArch64] Handle arrays of struct properly (from IR)

This only applies to FastIsel. GlobalIsel seems to sidestep
the issue.

This fixes https://bugs.llvm.org/show_bug.cgi?id=46996

One of the things we do in llvm is decide if a type needs
consecutive registers. Previously, we just checked if it
was an array or not.
(plus an SVE specific check that is not changing here)

This causes some confusion when you arbitrary IR like:
```
%T1 = type { double, i1 };
define [ 1 x %T1 ] @foo() {
entry:
  ret [ 1 x %T1 ] zeroinitializer
}
```

We see it is an array so we call CC_AArch64_Custom_Block
which bails out when it sees the i1, a type we don't want
to put into a block.

This leaves the location of the double in some kind of
intermediate state and leads to odd codegen. Which then crashes
the backend because it doesn't know how to implement
what it's been asked for.

You get this:
```
  renamable $d0 = FMOVD0
  $w0 = COPY killed renamable $d0
```

Rather than this:
```
  $d0 = FMOVD0
  $w0 = COPY $wzr
```

The backend knows how to copy 64 bit to 64 bit registers,
but not 64 to 32. It can certainly be taught how but the real
issue seems to be us even trying to assign a register block
in the first place.

This change makes the logic of
AArch64TargetLowering::functionArgumentNeedsConsecutiveRegisters
a bit more in depth. If we find an array, also check that all the
nested aggregates in that array have a single member type.

Then CC_AArch64_Custom_Block's assumption of a type that looks
like [ N x type ] will be valid and we get the expected codegen.

New tests have been added to exercise these situations. Note that
some of the output is not ABI compliant. The aim of this change is
to simply handle these situations and not to make our processing
of arbitrary IR ABI compliant.

Reviewed By: efriedma

Differential Revision: https://reviews.llvm.org/D104123