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