[PATCH] D35007: [PowerPC] Do not emit displacements for DQ-Form instructions that aren't multiples of 16

Hal Finkel via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Jul 6 18:25:48 PDT 2017


hfinkel added inline comments.


================
Comment at: lib/Target/PowerPC/PPCISelDAGToDAG.cpp:3022
+  SDValue AddrOp;
+  if (LDN)
+    AddrOp = LDN->getOperand(1);
----------------
Please write this as:

SDValue AddrOp = cast<MemSDNode>(N)->getBasePtr();



================
Comment at: lib/Target/PowerPC/PPCInstrInfo.td:408
+// instructions).
+def aligned16load : PatFrag<(ops node:$ptr), (load node:$ptr), [{
+  return isOffsetMultipleOf(N, 16);
----------------
nemanjai wrote:
> hfinkel wrote:
> > nemanjai wrote:
> > > If this solution is the way we want to proceed, perhaps it would be good to use the same approach for DS-Form instructions. These ultimately don't need to be aligned, they just can't have a displacement that isn't a multiple of 4.
> > Why can't we just check `cast<LoadSDNode>(N)->getAlignment() >= 16;` like we do above?
> But we are not actually interested in alignment. These instructions do not have an alignment requirement. The only issue is that the DQ field encodes a quad-word offset (and what we're working with here is a byte offset).
> 
> I can certainly see that the names are misleading and would be happy to change them. The only reason I didn't is that it seems like we might want to have some consistency with the above fragments.
> 
> The test case I added illustrates the intent. It comes from this:
> ```
> void test1(int *arr, int *arrTo) {
>   vector int *ptr = (vector int *)(&arrTo[4]);
>   *ptr = *(vector int*)(&arr[4]);
> }
> void test2(int *arr, int *arrTo) {
>   vector int *ptr = (vector int *)(&arrTo[1]);
>   *ptr = *(vector int*)(&arr[2]);
> }
> ```
> The `test1` function can use the DQ-Form instructions. The `test2` function cannot. And the alignment on the loads/stores is the same.
> I can certainly see that the names are misleading and would be happy to change them.

Yes, please do.


Repository:
  rL LLVM

https://reviews.llvm.org/D35007





More information about the llvm-commits mailing list