[PATCH] [SeparateConstOffsetFromGEP] Bail there is only one GEP offset
James Molloy
james at jamesmolloy.co.uk
Tue Apr 21 11:17:29 PDT 2015
Hi Jingue,
Hmm, I see. I'm on my mobile now so can't paste them, but the tests
modified as part of this diff are the examples in question.
I'll take another, less suspicious look tomorrow.
Cheers,
James
On Tue, 21 Apr 2015 at 19:15, Jingyue Wu <jingyue at google.com> wrote:
> 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/743b663a/attachment.html>
More information about the llvm-commits
mailing list