[llvm] r323371 - [AArch64][GlobalISel] Fall back during AArch64 isel if we have a volatile load.

[Amara Emerson via llvm-commits]

> On 25 Jan 2018, at 15:29, Hans Wennborg <hans at chromium.org> wrote:
> 
> OK, merged them in r323434.
> 
> On Wed, Jan 24, 2018 at 10:02 PM, Amara Emerson <aemerson at apple.com> wrote:
>>> On 24 Jan 2018, at 20:35, Amara Emerson via llvm-commits <llvm-commits at lists.llvm.org> wrote:
>>> 
>>> Author: aemerson
>>> Date: Wed Jan 24 12:35:37 2018
>>> New Revision: 323371
>>> 
>>> URL: http://llvm.org/viewvc/llvm-project?rev=323371&view=rev
>>> Log:
>>> [AArch64][GlobalISel] Fall back during AArch64 isel if we have a volatile load.
>>> 
>>> The tablegen imported patterns for sext(load(a)) don't check for single uses
>>> of the load or delete the original after matching. As a result two loads are
>>> left in the generated code. This particular issue will be fixed by adding
>>> support for a G_SEXTLOAD opcode in future.
>>> 
>>> There are however other potential issues around this that wouldn't be fixed by
>>> a G_SEXTLOAD, so until we have a proper solution we don't try to handle volatile
>>> loads at all in the AArch64 selector.
>>> 
>>> Fixes/works around PR36018.
>>> 
>>> Added:
>>>   llvm/trunk/test/CodeGen/AArch64/GlobalISel/irtranslator-volatile-load-pr36018.ll
>>> Modified:
>>>   llvm/trunk/lib/Target/AArch64/AArch64InstructionSelector.cpp
>>> 
>>> Modified: llvm/trunk/lib/Target/AArch64/AArch64InstructionSelector.cpp
>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/AArch64/AArch64InstructionSelector.cpp?rev=323371&r1=323370&r2=323371&view=diff
>>> ==============================================================================
>>> --- llvm/trunk/lib/Target/AArch64/AArch64InstructionSelector.cpp (original)
>>> +++ llvm/trunk/lib/Target/AArch64/AArch64InstructionSelector.cpp Wed Jan 24 12:35:37 2018
>>> @@ -931,6 +931,12 @@ bool AArch64InstructionSelector::select(
>>>      return false;
>>>    }
>>> 
>>> +    // FIXME: PR36018: Volatile loads in some cases are incorrectly selected by
>>> +    // folding with an extend. Until we have a G_SEXTLOAD solution bail out if
>>> +    // we hit one.
>>> +    if (Opcode == TargetOpcode::G_LOAD && MemOp.isVolatile())
>>> +      return false;
>>> +
>>>    const unsigned PtrReg = I.getOperand(1).getReg();
>>> #ifndef NDEBUG
>>>    const RegisterBank &PtrRB = *RBI.getRegBank(PtrReg, MRI, TRI);
>>> 
>>> Added: llvm/trunk/test/CodeGen/AArch64/GlobalISel/irtranslator-volatile-load-pr36018.ll
>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/AArch64/GlobalISel/irtranslator-volatile-load-pr36018.ll?rev=323371&view=auto
>>> ==============================================================================
>>> --- llvm/trunk/test/CodeGen/AArch64/GlobalISel/irtranslator-volatile-load-pr36018.ll (added)
>>> +++ llvm/trunk/test/CodeGen/AArch64/GlobalISel/irtranslator-volatile-load-pr36018.ll Wed Jan 24 12:35:37 2018
>>> @@ -0,0 +1,14 @@
>>> +; RUN: llc -O0 -mtriple=aarch64-apple-ios -o - %s | FileCheck %s
>>> +
>>> + at g = global i16 0, align 2
>>> +declare void @bar(i32)
>>> +
>>> +; Check that only one load is generated. We fall back to
>>> +define hidden void @foo() {
>>> +; CHECK-NOT: ldrh
>>> +; CHECK: ldrsh
>>> +  %1 = load volatile i16, i16* @g, align 2
>>> +  %2 = sext i16 %1 to i32
>>> +  call void @bar(i32 %2)
>>> +  ret void
>>> +}
>>> 
>>> 
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>> 
>> Hi Hans,
>> 
>> This is a 6.0 blocker, can we have r323369 and this commit on the 6.0 branch please?
>> 
>> Thanks,
>> Amara

Could we also have r323384? It’s just a tweak to a test to skip on non-asserts builds.

Thanks,
Amara