This is an archive of the discontinued LLVM Phabricator instance.

[SROA] Remove UB-implying metadata when promoting speculative instruction.
ClosedPublic

Authored by DianQK on Apr 16 2023, 1:45 AM.

Details

Summary

After D138238 introduced the then/else blocks, we should remove UB-implying metadata for the promoted speculative instruction.

This patch fixes the LTO=thin appears broken on nightly rustc? #110256.

Diff Detail

Event Timeline

DianQK created this revision.Apr 16 2023, 1:45 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 16 2023, 1:45 AM
Herald added a subscriber: hiraditya. · View Herald Transcript
DianQK requested review of this revision.Apr 16 2023, 1:45 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 16 2023, 1:45 AM
DianQK edited the summary of this revision. (Show Details)Apr 16 2023, 1:57 AM
DianQK added reviewers: lebedev.ri, arsenm, nikic.
StephenFan added a comment.EditedApr 16 2023, 2:07 AM

It seems that !noundef means that the loaded value is not an undefined value instead of loading a value from an under ptr is UB.

DianQK edited the summary of this revision. (Show Details)Apr 16 2023, 2:27 AM
DianQK added a reviewer: StephenFan.

It seems that !noundef means that the loaded value is not an undefined value instead of loading a value from an under ptr is UB.

Thanks, I tried to correct the summary.

nikic requested changes to this revision.Apr 16 2023, 3:03 AM

You are looking for dropUBImplyingAttrsAndUnknownMetadata().

This revision now requires changes to proceed.Apr 16 2023, 3:03 AM
nikic added a comment.Apr 16 2023, 3:05 AM

Well, actually you are looking for dropUBImplyingAttrsAndMetadata() from D146629, but for now you can just drop all unknown metadata, and we can add the UB implying distinction later.

DianQK updated this revision to Diff 513993.EditedApr 16 2023, 5:18 AM

Promote the load instruction by calling dropUBImplyingAttrsAndMetadata.

Thanks for the reminder that dropUBImplyingAttrsAndMetadata()/dropUBImplyingAttrsAndUnknownMetadata() is the method I was looking for.

DianQK retitled this revision from [SROA] Unconditionally load is unsafe with nonnull and noundef metadata. to [SROA] Remove UB-implying metadata when promoting speculative instruction..Apr 16 2023, 5:26 AM
DianQK edited the summary of this revision. (Show Details)
nikic accepted this revision.Apr 16 2023, 7:04 AM

LG

llvm/test/Transforms/SROA/select-load-with-nonnull.ll
2

I'd suggest adding this test to llvm/test/Transforms/SROA/select-load.ll instead.

This revision is now accepted and ready to land.Apr 16 2023, 7:04 AM
DianQK updated this revision to Diff 514005.EditedApr 16 2023, 7:14 AM
DianQK edited the summary of this revision. (Show Details)

Move the test to llvm/test/Transforms/SROA/select-load.ll.

Maybe we should also cherry-pick to 16.0.2?

This revision was landed with ongoing or failed builds.Apr 16 2023, 7:37 AM
This revision was automatically updated to reflect the committed changes.