[PATCH] D60294: [DAGCombiner] [CodeGenPrepare] WIP/RFC Splitting large offsets from base addresses

Luís Marques via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Apr 4 16:44:13 PDT 2019


luismarques created this revision.
luismarques added reviewers: efriedma, spatel, asb.
Herald added subscribers: llvm-commits, jocewei, PkmX, the_o, brucehoult, MartinMosbeck, rogfer01, edward-jones, zzheng, jrtc27, niosHD, sabuasal, apazos, simoncook, johnrusso, rbar, kristof.beyls, tpr, javed.absar.
Herald added a project: LLVM.

This patch is a WIP solution (and a RFC) to address the following issue that came up in the context of RISC-V benchmarks, but which affects other targets. Suppose you have several loads/stores that access array elements or struct fields with large offsets:

void foo(int *x, int *y) {

  y[0] = x[0x10001];
  y[1] = x[0x10002];
  y[2] = x[0x10003];
  ...

}

In a target such as RISC-V you cannot add 0x10001 to the address of X in a single instruction (the constant doesn't fit the 12-bit signed immediate), so the generated code is more directly reflected by something like this:

void foo(int *x, int *y) {

  y[0] = *(x+0x10000+1);
  y[1] = *(x+0x10000+2);
  y[2] = *(x+0x10000+3);
  ...

}

But you can fold the +1, etc. into an immediate offset of the load/store instructions, so you are able to effectively have something like this:

void foo(int *x, int *y) {

  int *base = &x[0x10001];
  y[0] = base[0];
  y[1] = base[1];
  y[2] = base[2];
  ...

}

That optimization (as it stands) is only able to be performed, though, if the +1, +2, etc. are split from the 0x10000. Fortunately, there is already a target hook that indicates we want such an address split to occur: shouldConsiderGEPOffsetSplit. When that hook returns true, CodeGenPrepare.cpp adds the GEPs with large offsets to a list of GEPs to be split and ::splitLargeGEPOffsets splits them, in a process clearly illustrated in that method's comments. Unfortunately, the split currently only occurs when the base and the GEP are in different BBs, since the DAGCombiner would just recombine those in the same BB anyway.

This patch tries to 1) make the split also occur in the cases where the base and the GEP are in the same BB (that's often the case); and 2) ensure that the DAGCombiner doesn't reassociate them back again. To achieve that second step the patch adds a check before the reassociation of add instructions to see if the added constants could match a reg+imm addressing mode. If they match before the constants are combined but not after then we skip the reassociation. For the most part this strategy seems to work, as shown in the new tests. Unfortunately, the patch breaks a few test cases:

Failing Tests (7):

  LLVM :: CodeGen/AMDGPU/global-saddr.ll
  LLVM :: CodeGen/SystemZ/int-add-08.ll
  LLVM :: CodeGen/SystemZ/int-sub-05.ll
  LLVM :: CodeGen/ARM/misched-fusion-aes.ll
  LLVM :: CodeGen/ARM/vector-spilling.ll
  LLVM :: CodeGen/AArch64/global-merge-3.ll
  LLVM :: CodeGen/X86/zext-sext.ll

Some of these tests break because of the split GEPs and some because of the skipped (re)association of some add instructions (apparently such reassociation is an important canonicalization step?). The X86 failure is actually a crash, that occurs when performing the LEA fixups pass and a check for undef is performed on a non-register operand. The exact root causes of the failures are hard to pinpoint without a deep dive into the respective targets.

Before we spend even more effort trying to fix these failing tests, we would appreciate feedback on whether this is a viable/sound approach at all, or if this problem would be better tackled in another way. A possible alternative would be to add a RISC-V specific pass to split the addresses, but if this problem could be solved in a more generic fashion that would probably be preferable, as it would avoid duplication of functionality and possibly benefit other targets.


Repository:
  rL LLVM

https://reviews.llvm.org/D60294

Files:
  lib/CodeGen/CodeGenPrepare.cpp
  lib/CodeGen/SelectionDAG/DAGCombiner.cpp
  lib/Target/RISCV/RISCVISelLowering.h
  test/CodeGen/RISCV/split-offsets-1.ll
  test/CodeGen/RISCV/split-offsets-2.ll

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D60294.193789.patch
Type: text/x-patch
Size: 7680 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190404/adf5420f/attachment.bin>


More information about the llvm-commits mailing list