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

[Tim Northover via llvm-commits]

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
  [...]

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.

Trying to handle it in SDAG is possibly even worse, with the entire
DAG consisting of an ldaxr followed by "CopyToReg:vregNN".
Theoretically, I could delve into and rewrite that CopyToReg even
while I'm supposed to be selecting the ATOMIC_CMP_SWAP, but I think
that's a step too far in undermining the representation.

So the summary is that I couldn't get it to work.

> Should this also transferSuccessorsAndUpdatePHIs()?

Oh, good point (also noting your later amendment). This is why I hate
the late expansion...

> MF is already available in SelectionDAGISel.

I'll certainly give it a go, but I'm fairly sure I only added this
line after a compiler error. Actually, that might have been when the
code was in ISelLowering. Certainly worth a try!

> missing word: "to the,"

Thanks!

> mayLoad/mayStore here as well?

Yep, already fixed actually (a hangover from when I was able to write
patterns for them).

I'll try to get a new version uploaded over the weekend.

Tim.