This is an archive of the discontinued LLVM Phabricator instance.

[OpaquePtr] Make atomicrmw work with opaque pointers
ClosedPublic

Authored by aeubanks on May 19 2021, 10:52 AM.

Details

Summary

FullTy is only necessary when we need to figure out what type an
instruction works with given a pointer's pointee type. However, we just
end up using the value operand's type, so FullTy isn't necessary.

Diff Detail

Event Timeline

aeubanks created this revision.May 19 2021, 10:52 AM
aeubanks requested review of this revision.May 19 2021, 10:52 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 19 2021, 10:52 AM
dblaikie accepted this revision.May 19 2021, 11:15 AM

Looks good!

This revision is now accepted and ready to land.May 19 2021, 11:15 AM
This revision was automatically updated to reflect the committed changes.
bogner added a subscriber: bogner.May 25 2021, 12:02 PM

Looks like you've already reverted this, but I happened to bugpoint a test case for the failure if you happen to need it. The issue is with the call to popValue if the value the atomicrmw is using hasn't been seen by the parser yet.

; RUN: opt %s -o %t.bc
; RUN: opt %t.bc -o /dev/null

define void @f() {
entry:
  br label %def

use:
  %x = atomicrmw add i32 addrspace(1)* undef, i32 %y monotonic, align 4
  br label %exit

def:
  %y = add i32 undef, undef
  br i1 undef, label %use, label %exit

exit:
  ret void
}

thanks for the repro! I'll take a look at it

aeubanks reopened this revision.May 25 2021, 4:57 PM
This revision is now accepted and ready to land.May 25 2021, 4:57 PM
aeubanks updated this revision to Diff 347826.May 25 2021, 5:00 PM

rebase past D103123
add previously crashing test case

dblaikie accepted this revision.May 25 2021, 5:38 PM

Looks good, thanks!

llvm/test/Assembler/atomicrmw.ll
1–5 ↗(On Diff #347826)

Comment might be handy to describe what situation makes this interesting/how it failed previously, or the like?

aeubanks updated this revision to Diff 347838.May 25 2021, 8:16 PM

add test comment

This revision was landed with ongoing or failed builds.May 25 2021, 8:20 PM
This revision was automatically updated to reflect the committed changes.