[PATCH] D26367: Fix DAGCombiner match

[Evgeny Stupachenko via Phabricator via llvm-commits]

evstupac added a comment.

>> This is not a good result. It seems fairly obvious to me that two instructions are worse than one - an LDRH would be what I would expect generated.

I'm not Arm specialist.
The only similar test I've found is:
test/CodeGen/ARM/MergeConsecutiveStores.ll
But it it has an option -mtriple=armv7-apple-darwin. For the option I get the result I've posted. I don't want to modify it.
For -mtriple=aarch64 I get expected result:

  ldrh w9, [x2, x9]

But for Aarch64 there no other tests on store/load merge.
I can restructure all this how I feel is better.
But again I'm not Arm specialist. And (since there are no regressions) it is better leave all this for someone who has better knowledge in Arm.



================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:11261
       int64_t Offset = cast<ConstantSDNode>(Ptr->getOperand(1))->getSExtValue();
-      return  BaseIndexOffset(Ptr->getOperand(0), SDValue(), Offset,
-                              IsIndexSignExt);
+      return match(Ptr->getOperand(0), DAG, Offset + PartialOffset);
     }
----------------
jmolloy wrote:
> Why has IsIndexSignExt been removed here?
It was not removed. We'll add it when form return value BaseIndexOffset(..,..,..).
Here we just do not stop when found VAR + OFFSET, but recursively call match for the VAR. It could happen that VAR is not just BASE, but BASE + INDEX. 


Repository:
  rL LLVM

https://reviews.llvm.org/D26367