Page MenuHomePhabricator

[SelectionDAG][Mips][PowerPC][RISCV][WebAssembly] Teach computeKnownBits/ComputeNumSignBits about atomics
ClosedPublic

Authored by jrtc27 on Mon, Apr 26, 9:33 PM.

Details

Summary

Unlike normal loads these don't have an extension field, but we know
from TargetLowering whether these are sign-extending or zero-extending,
and so can optimise away unnecessary extensions.

This was noticed on RISC-V, where sign extensions in the calling
convention would result in unnecessary explicit extension instructions,
but this also fixes some Mips inefficiencies. PowerPC sees churn in the
tests as all the zero extensions are only for promoting 32-bit to
64-bit, but these zero extensions are still not optimised away as they
should be, likely due to i32 being a legal type.

This also simplifies the WebAssembly code somewhat, which currently
works around the lack of target-independent combines with some ugly
patterns that break once they're optimised away.

Diff Detail

Event Timeline

jrtc27 created this revision.Mon, Apr 26, 9:33 PM
jrtc27 requested review of this revision.Mon, Apr 26, 9:33 PM
Herald added a project: Restricted Project. · View Herald TranscriptMon, Apr 26, 9:33 PM
jrtc27 added inline comments.Mon, Apr 26, 9:38 PM
llvm/test/CodeGen/RISCV/atomic-signext.ll
19

NB: These tests aren't in the tree, this is just showing the diff to my local pre-commit.

The SelectionDAG changes LGTM.

@jrtc27 Please can you pre-commit your local tests and then rebase this patch?

jrtc27 updated this revision to Diff 342686.Tue, May 4, 3:14 AM

Rebased with tests pre-committed

RKSimon accepted this revision.Tue, May 4, 3:59 AM

LGTM - please can you raise a bug detailing the PPC poor codegen for i32/i64 promotions

This revision is now accepted and ready to land.Tue, May 4, 3:59 AM
jrtc27 added a comment.Tue, May 4, 4:08 AM

LGTM - please can you raise a bug detailing the PPC poor codegen for i32/i64 promotions

Filed as https://bugs.llvm.org/show_bug.cgi?id=50217. Does this need a review from a WASM person given the changes there?

RKSimon added a subscriber: tlively.Tue, May 4, 4:24 AM

@tlively @sunfish Any final comments on the wasm change?

The Wasm changes LGTM! This is a nice simplification.

atanasyan accepted this revision.Wed, May 5, 1:31 AM

The MIPS changes LGTM

This revision was landed with ongoing or failed builds.Wed, May 5, 8:35 AM
This revision was automatically updated to reflect the committed changes.
jrtc27 reopened this revision.Wed, May 5, 9:02 AM

Reverted due to assertion failures during sanitiser builds on the buildbots. Will try and reproduce+debug over the coming days.

This revision is now accepted and ready to land.Wed, May 5, 9:02 AM

LGTM - please can you raise a bug detailing the PPC poor codegen for i32/i64 promotions

Filed as https://bugs.llvm.org/show_bug.cgi?id=50217. Does this need a review from a WASM person given the changes there?

Thanks for opening the bug. This is a fairly long standing issue in the PPC back end and isn't specific to atomics. We do plan to address it when we find time.

This revision was automatically updated to reflect the committed changes.