Page MenuHomePhabricator

[selectionDAG][atomics] Add sign extension handling to atom cmp and swap
ClosedPublic

Authored by sdardis on Mar 22 2016, 9:42 AM.

Details

Reviewers
t.p.northover
Summary

ATOMIC_CMP_AND_SWAP_WITH_SUCCESS needs to generate sign extension nodes for
targets that sign extend their implementation of ATOMIC_CMP_AND_SWAP.

This is required as MIPS canonicalises the result of atomic operations into sign extended
form. The resulting setCC can then have ATOMIC_CMP_AND_SWAP operand which is
signed and an operand which may be unsigned.

Diff Detail

Event Timeline

sdardis updated this revision to Diff 51292.Mar 22 2016, 9:42 AM
sdardis retitled this revision from to [selectionDAG][atomics] Add sign extension handling to atom cmp and swap.
sdardis updated this object.
t.p.northover accepted this revision.Mar 22 2016, 9:50 AM
t.p.northover added a reviewer: t.p.northover.

Thanks for the generic solution Simon, this looks good to me.

Tim.

This revision is now accepted and ready to land.Mar 22 2016, 9:50 AM

Can you commit on my behalf? Thanks.

This doesn't fix the 32 bit on 64 bit case.

For 16 on 32bit:

Legalizing: t22: i32,i32,ch = AtomicCmpSwapWithSuccess<Volatile LDST2[%addr]> t11, t2, t4, t6
 ... replacing: t22: i32,i32,ch = AtomicCmpSwapWithSuccess<Volatile LDST2[%addr]> t11, t2, t4, t6
     with:      t27: i32 = AssertSext t25, ValueType:ch:i16
      and:      t30: i32 = setcc t27, t28, seteq:ch
      and:      t25: i32,ch = AtomicCmpSwap<Volatile LDST2[%addr]> t11, t2, t4, t6

Which is what is required. But for 32 on 64bit I'm getting:

Legalizing: t27: i32,i32,ch = AtomicCmpSwapWithSuccess<Volatile LDST4[%addr]> t18, t2, t7, t15
 ... replacing: t27: i32,i32,ch = AtomicCmpSwapWithSuccess<Volatile LDST4[%addr]> t18, t2, t7, t15
     with:      t30: i32,ch = AtomicCmpSwap<Volatile LDST4[%addr]> t18, t2, t7, t15
      and:      t32: i32 = setcc t30, t7, seteq:ch
      and:      t30: i32,ch = AtomicCmpSwap<Volatile LDST4[%addr]> t18, t2, t7, t15

This is after adding:

if (Subtarget.isGP64bit()) {
  AddPromotedToType(ISD::ATOMIC_CMP_SWAP_WITH_SUCCESS, MVT::i32, MVT::i64);
  AddPromotedToType(ISD::ATOMIC_CMP_SWAP, MVT::i32, MVT::i64);
}

To MipsISelLowering.cpp. Any suggestions?

I was wrong, sorry for the noise.

Oh good, I was just trying to work out what was going wrong. Anyway, it's committed as r264296. Sorry about the delay and thanks for working on it!

Tim.

sdardis closed this revision.Mar 24 2016, 2:44 PM

Thanks again.