[PATCH] D16239: Fix PR25526 by adding back limited cmpxchg pseudo-Insts

Ahmed Bougacha via llvm-commits llvm-commits at lists.llvm.org
Fri Jan 15 20:25:06 PST 2016


On Fri, Jan 15, 2016 at 6:30 PM, Tim Northover <t.p.northover at gmail.com> wrote:
> Thanks for taking a look Ahmed.
>
> On 15 January 2016 at 17:58, Ahmed Bougacha <ahmed.bougacha at gmail.com> wrote:
>>> Handle the expanded @llvm.aarch64.ldxr/stxr intrinsics to do away with any vregs (so fast-regalloc can't botch them), either in DAG or FastISel. I tried both, but the IR comparison always creates more. No change.
>>
>> I don't understand: what "IR comparison"? If it worked, this seems to me like the ideal solution.
>
> I was really hopeful when the idea occurred to me too. Unfortunately
> the incoming IR we receive (at FastISel or DAG) is:
>
> cmpxchg.start:                                    ; preds =
> %cmpxchg.trystore, %0
>   %1 = call i64 @llvm.aarch64.ldaxr.p0i64(i64* %addr)
>   %should_store = icmp eq i64 %1, %desired
>   br i1 %should_store, label %cmpxchg.trystore, label %cmpxchg.nostore
>   [...]

Ah yes, of course, sorry! I was imagining the DAG after disabling the
expansion, with cmpxchg intact. But then the only thing you can do is
the pseudo, so back to square one.

> When FastISel is involved, no matter how explicit I make the "ldaxr"
> (and I materialised it and the corresponding stlxr with a BuildMI
> using not a single VReg, hopeful I could fix up any mess later),
> generic code gets invoked to handle the "icmp" and creates a VReg,
> which gets spilled at the end of the BB.

So, I cringe as I'm typing this, but what about using an intrinsic for
the comparison as well?

-Ahmed


More information about the llvm-commits mailing list