[AArch64] Respect pre-/post-decrement indexing mode during instruction selection

[momo5502]

Summary

Loads with pre-decrement and post-decrement indexing modes are lowered as pre-increment and post-increment loads on AArch64.
The decrement-semantics are ignored, which results in incorrect addressing.

[momo5502]

Here's a bit more context:

I have a sample, which, unfortunately, I am not able to disclose. In one basicblock, the following transformation is made in SelectionDAG:

What's essentially happening here is that the following expression:

t54 = t53 - 8
load(t54)

is transformed to:

load<pre-dec>(t53, 8). Due to the pre-dec indexing mode it seems like a valid transformation.

However, during instruction selection, the load is transformed like this:

LDRXpre(t53, 8) is selected which is then lowered to ldr x24, [x2, #8]!.
Essentially transforming the offset -8 into a +8. Which is simply wrong.
The reason is, as stated above, that dec indexing modes are not respected during instruction selection on AArch64.

Usually, patterns like this don't occur. The expression t54 = t53 - 8 is usually transformed to t54 = t53 + (-8) in prior optimization/transformation steps.
This can then be represented using increment indexing.

For unknown reasons, this is not the case with my sample.

Here is a drastically reduced version of the initial sample (which is still pretty big 🙁), that shows the issue: https://godbolt.org/z/sn5n9r96v One can see ldr x24, [x2, #8]! in line 65, which should instead be a -8.
However, the sample is highly sensitive to changes. The slightest adaptation will not trigger the bug anymore. Which is why I was not able to reduce it any further.

It is to note that switching to llc (trunk) yields the correct result. The reason for that is that the subtraction is previously transformed into an addtion, which is not represented as a decrement load anymore. Again, that is because the slightest change won't trigger the bug anymore.

This makes it extremely difficult to design a test case.
I tried reducing my initial sample as much as possible, however, the semantics in that godbolt snippet are meaningless and it is still way too big, which does not make for a good test, in my opinion.
Ontop of that, in my specific case, the issue does not occur anymore with the latest version of llc.

However, I still feel like the initial issue should be fixed, hence this commit.

[efriedma]

I think with fb7c3807, we shouldn't use PRE_DEC/POST_DEC modes anymore. So IsDec will always be false, and this patch does nothing.

[momo5502]

In D156396#4539673, @efriedma wrote:

I think with fb7c3807, we shouldn't use PRE_DEC/POST_DEC modes anymore. So IsDec will always be false, and this patch does nothing.

Ah man, ngl, that hurts. 3 days of straight debugging just to see this pattern cannot occur anymore. I guess it do be like that sometimes 😂 Still thank you though.

llvm/lib/Target/AArch64/AArch64ISelDAGToDAG.cpp

 bool AArch64DAGToDAGISel::tryIndexedLoad(SDNode *N) {
   LoadSDNode *LD = cast<LoadSDNode>(N);
   if (LD->isUnindexed())
     return false;
   EVT VT = LD->getMemoryVT();
   EVT DstVT = N->getValueType(0);
   ISD::MemIndexedMode AM = LD->getAddressingMode();
   bool IsPre = AM == ISD::PRE_INC || AM == ISD::PRE_DEC;
+  bool IsDec = AM == ISD::PRE_DEC || AM == ISD::POST_DEC;
 
   // We're not doing validity checking here. That was done when checking
   // if we should mark the load as indexed or not. We're just selecting
   // the right instruction.
   unsigned Opcode = 0;
 
   ISD::LoadExtType ExtType = LD->getExtensionType();
   bool InsertTo64 = false;
bool AArch64DAGToDAGISel::tryIndexedLoad(SDNode *N) {
   } else if (VT.is128BitVector()) {
     Opcode = IsPre ? AArch64::LDRQpre : AArch64::LDRQpost;
   } else
     return false;
   SDValue Chain = LD->getChain();
   SDValue Base = LD->getBasePtr();
   ConstantSDNode *OffsetOp = cast<ConstantSDNode>(LD->getOffset());
   int OffsetVal = (int)OffsetOp->getZExtValue();
+  OffsetVal = IsDec ? -OffsetVal : OffsetVal;
   SDLoc dl(N);
   SDValue Offset = CurDAG->getTargetConstant(OffsetVal, dl, MVT::i64);
   SDValue Ops[] = { Base, Offset, Chain };
   SDNode *Res = CurDAG->getMachineNode(Opcode, dl, MVT::i64, DstVT,
                                        MVT::Other, Ops);
 
   // Transfer memoperands.
   MachineMemOperand *MemOp = cast<MemSDNode>(N)->getMemOperand();