[PATCH] [SeparateConstOffsetFromGEP] Bail there is only one GEP offset

[Jingyue Wu]

For example,

p1 = &base[offset + 1];
p2 = &base[offset + 2];

Reassociating GEPs gives us

p1 = (&base[offset]) + 1
p2 = (&base[offset]) + 2

where &base[offset] can be CSE'ed.

Can you show me the code patterns you discovered that demonstrates the
regression? I can think about how to address them too. Thanks!

Jingyue

On Tue, Apr 21, 2015 at 11:10 AM James Molloy <james at jamesmolloy.co.uk>
wrote:

> Hi Jingue,
>
> What could %offset be re associated with? It's just one variable- ok it
> could be multiplied by something, but the only real operation you can do on
> it is strength reduction.
>
> Enabling it pessimizes a bunch of code by creating unnecessary ptrtoints
> that the optimiser isn't as good at handling as GEPs. As a matter of fact,
> this patch is reenabling some aarch64 tests that currently fail.
>
> Can you give an example of when this would be useful?
> On Tue, 21 Apr 2015 at 17:45, David Blaikie <dblaikie at gmail.com> wrote:
>
>> On Tue, Apr 21, 2015 at 6:54 AM, James Molloy <james.molloy at arm.com>
>> wrote:
>>
>>> Hi jingyue, wu, Gerolf,
>>>
>>> A single-offset GEP such as:
>>>
>>>   getelementptr i8*, i8* %base, i32 %offset
>>>
>>
>> Just out of pedantry/by way of explanation, that should be: getelementptr
>> i8, i8* %base, ....
>>
>> (note the first type parameter is the type pointed to, not the pointer
>> type - this is part of the transition to opaque pointer types - so there
>> will be no "pointer to i8" in the future, just a raw pointer and operations
>> like gep will indicate how to treat the referenced bytes)
>>
>>
>>>
>>> doesn't make sense to split apart. There is no complex calculation to
>>> CSE.
>>>
>>> REPOSITORY
>>>   rL LLVM
>>>
>>> http://reviews.llvm.org/D9149
>>>
>>> Files:
>>>   lib/Transforms/Scalar/SeparateConstOffsetFromGEP.cpp
>>>   test/CodeGen/AArch64/arm64-addr-mode-folding.ll
>>>   test/CodeGen/AArch64/arm64-cse.ll
>>>   test/Transforms/SeparateConstOffsetFromGEP/NVPTX/split-gep.ll
>>>
>>> Index: lib/Transforms/Scalar/SeparateConstOffsetFromGEP.cpp
>>> ===================================================================
>>> --- lib/Transforms/Scalar/SeparateConstOffsetFromGEP.cpp
>>> +++ lib/Transforms/Scalar/SeparateConstOffsetFromGEP.cpp
>>> @@ -829,7 +829,7 @@
>>>
>>>    // The backend can already nicely handle the case where all indices
>>> are
>>>    // constant.
>>> -  if (GEP->hasAllConstantIndices())
>>> +  if (GEP->hasAllConstantIndices() || GEP->getNumOperands() == 2)
>>>      return false;
>>>
>>>    bool Changed = canonicalizeArrayIndicesToPointerSize(GEP);
>>> Index: test/CodeGen/AArch64/arm64-addr-mode-folding.ll
>>> ===================================================================
>>> --- test/CodeGen/AArch64/arm64-addr-mode-folding.ll
>>> +++ test/CodeGen/AArch64/arm64-addr-mode-folding.ll
>>> @@ -1,4 +1,4 @@
>>> -; RUN: llc -O3 -mtriple arm64-apple-ios3 -aarch64-gep-opt=false %s -o -
>>> | FileCheck %s
>>> +; RUN: llc -O3 -mtriple arm64-apple-ios3 %s -o - | FileCheck %s
>>>  ; <rdar://problem/13621857>
>>>
>>>  @block = common global i8* null, align 8
>>> Index: test/CodeGen/AArch64/arm64-cse.ll
>>> ===================================================================
>>> --- test/CodeGen/AArch64/arm64-cse.ll
>>> +++ test/CodeGen/AArch64/arm64-cse.ll
>>> @@ -1,4 +1,4 @@
>>> -; RUN: llc -O3 < %s -aarch64-atomic-cfg-tidy=0 -aarch64-gep-opt=false
>>> -verify-machineinstrs | FileCheck %s
>>> +; RUN: llc -O3 < %s -aarch64-atomic-cfg-tidy=0 -verify-machineinstrs |
>>> FileCheck %s
>>>  target triple = "arm64-apple-ios"
>>>
>>>  ; rdar://12462006
>>> @@ -12,7 +12,7 @@
>>>  ; CHECK-NOT: sub
>>>  ; CHECK: b.ge
>>>  ; CHECK: sub
>>> -; CHECK: sub
>>> +; CHECK: {{sub|add}}
>>>  ; CHECK-NOT: sub
>>>  ; CHECK: ret
>>>   %0 = load i32, i32* %offset, align 4
>>> Index: test/Transforms/SeparateConstOffsetFromGEP/NVPTX/split-gep.ll
>>> ===================================================================
>>> --- test/Transforms/SeparateConstOffsetFromGEP/NVPTX/split-gep.ll
>>> +++ test/Transforms/SeparateConstOffsetFromGEP/NVPTX/split-gep.ll
>>> @@ -237,7 +237,7 @@
>>>
>>>  ; The code that rebuilds an OR expression used to be buggy, and failed
>>> on this
>>>  ; test.
>>> -define float* @shl_add_or(i64 %a, float* %ptr) {
>>> +define float* @shl_add_or(i64 %a, [10 x float]* %ptr) {
>>>  ; CHECK-LABEL: @shl_add_or(
>>>  entry:
>>>    %shl = shl i64 %a, 2
>>> @@ -247,8 +247,8 @@
>>>    ; ((a << 2) + 12) and 1 have no common bits. Therefore,
>>>    ; SeparateConstOffsetFromGEP is able to extract the 12.
>>>    ; TODO(jingyue): We could reassociate the expression to combine 12
>>> and 1.
>>> -  %p = getelementptr float, float* %ptr, i64 %or
>>> -; CHECK: [[PTR:%[a-zA-Z0-9]+]] = getelementptr float, float* %ptr, i64
>>> [[OR]]
>>> +  %p = getelementptr [10 x float], [10 x float]* %ptr, i64 %or, i64 0
>>> +; CHECK: [[PTR:%[a-zA-Z0-9]+]] = getelementptr [10 x float], [10 x
>>> float]* %ptr, i64 [[OR]], i64 0
>>>  ; CHECK: getelementptr float, float* [[PTR]], i64 12
>>>    ret float* %p
>>>  ; CHECK-NEXT: ret
>>>
>>> EMAIL PREFERENCES
>>>   http://reviews.llvm.org/settings/panel/emailpreferences/
>>>
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>>
>>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150421/6237a235/attachment.html>