PowerPC: cmpxchg generates incorrect code for i16 and i8

[llvmbot]

Bugzilla Link	35812
Resolution	FIXED
Resolved on	Jan 24, 2018 07:33
Version	trunk
OS	Linux
Blocks	#35152
Attachments	reproducer for bad code gen
Reporter	LLVM Bugzilla Contributor
CC	@zmodem,@hfinkel,@nemanjai
Fixed by commit(s)	https://reviews.llvm.org/rL322372

Extended Description

On PowerPC:

% clang -o a.out bug.bc
% ./a.out
FAILED: __atomic_compare_exchange_n() failed.
%

On x86:

% clang -o a.out bug.bc
warning: overriding the module target triple with x86_64-unknown-linux-gnu
[-Woverride-module]
1 warning generated.
% ./a.out
PASSED
%

[llvmbot]

assigned to @nemanjai

[nemanjai]

Ah yeah, I'll fix this. The problem is that lharx doesn't sign extend whereas cmplwi does.

Thanks for opening this bug.

[nemanjai]

The fix (target-specific now) is in https://reviews.llvm.org/D41856.

[nemanjai]

The feedback on the patch was to not encapsulate all of this in the existing atomic SDNode's but to create PPC-specific ones. The committed incarnation of the aforementioned patch is therefore not a complete fix and relies on https://reviews.llvm.org/D41798 being committed as well.

Once the latter is committed, I will ask to port both of those to 6.0.

[nemanjai]

This is fixed by a pair of commits:
https://reviews.llvm.org/rL322767
https://reviews.llvm.org/rL322372

These two commits need to be merged into the 6.0 branch. Hans, are you able to do that or do I need to get further approval and commit it to the branch?

[zmodem]

This is fixed by a pair of commits:
https://reviews.llvm.org/rL322767
https://reviews.llvm.org/rL322372

These two commits need to be merged into the 6.0 branch. Hans, are you able
to do that or do I need to get further approval and commit it to the branch?

Let's keep this open until it's merged.

Hal, does this sound good for merging?

[hfinkel]

This is fixed by a pair of commits:
https://reviews.llvm.org/rL322767
https://reviews.llvm.org/rL322372

These two commits need to be merged into the 6.0 branch. Hans, are you able
to do that or do I need to get further approval and commit it to the branch?

Let's keep this open until it's merged.

Hal, does this sound good for merging?

Yes, please do.

[zmodem]

Merged in r323334.