This is an archive of the discontinued LLVM Phabricator instance.

Revert of D49126 [PredicateInfo] Use custom mangling to support ssa_copy with unnamed types.
ClosedPublic

Authored by jeroen.dobbelaere on Nov 17 2020, 2:28 PM.

Details

Summary

Now that intrinsic name mangling can cope with unnamed types, the custom name mangling in PredicateInfo (introduced by D49126) can be removed.
(See D91250, D48541)

Diff Detail

Event Timeline

Herald added a project: Restricted Project. · View Herald TranscriptNov 17 2020, 2:28 PM
jeroen.dobbelaere edited the summary of this revision. (Show Details)Nov 17 2020, 2:35 PM

Omitted reversal of NewGVN testcase that slipped in with the original (D49126) commit. It has nothing to do with the unnamed type support.

fhahn added inline comments.Mar 10 2021, 12:19 PM
llvm/test/Transforms/Util/PredicateInfo/testandor.ll
232

do you know why the order changed here?

877

no need to have {{.+}} here any longer?

llvm/test/Transforms/Util/PredicateInfo/testandor.ll
232

Good that you asked. The reverted code now also reverses some of the insertions. Working on a fix.

Fixed an unwanted reversal of instructions shown in the testandor.ll test.

jeroen.dobbelaere marked 2 inline comments as done.Mar 10 2021, 2:23 PM
fhahn accepted this revision.Mar 14 2021, 3:08 PM

LGTM, thanks!

llvm/lib/Transforms/Utils/PredicateInfo.cpp
763–764

nit: move to the header?

This revision is now accepted and ready to land.Mar 14 2021, 3:08 PM

Rebased. Moved ~PredicateInfo() to header.

This patch is causing issues with the EXPENSIVE_CHECKS build, which detects that the pass modified the IR without reporting it has done so via runOnFunction's return value. Would you mind taking a look?

llvm/test/Transforms/Util/PredicateInfo/testandor.ll

Pass modifies its input and doesn't report it: PredicateInfo Printer
Pass modifies its input and doesn't report it
UNREACHABLE executed at llvm/lib/IR/LegacyPassManager.cpp:1445!
PLEASE submit a bug report to https://bugs.llvm.org/ and include the crash backtrace.
Stack dump:

This patch is causing issues with the EXPENSIVE_CHECKS build, which detects that the pass modified the IR without reporting it has done so via runOnFunction's return value. Would you mind taking a look?

llvm/test/Transforms/Util/PredicateInfo/testandor.ll

Pass modifies its input and doesn't report it: PredicateInfo Printer
Pass modifies its input and doesn't report it
UNREACHABLE executed at llvm/lib/IR/LegacyPassManager.cpp:1445!
PLEASE submit a bug report to https://bugs.llvm.org/ and include the crash backtrace.
Stack dump:

Thanks for the notificiation. I'll take a look at it.

@jroelofs I tried to run llvm/test/Transforms/Util/PredicateInfo/testandor.ll for both rG77080a1eb6061df2dcfae8ac84b85ad4d1e02031 (the moment of the commit) and rGfb9c5c3dce27b352534641dbb6e3cb8c05da7bc9 (recent main), with -DLLVM_ENABLE_EXPENSIVE_CHECKS=1.
I checked that the -DEXPENSIVE_CHECKS is added to the ninja.build file. But, I am not able to observe the issue you are mentioning. How can I reproduce this problem ?

fhahn added a comment.Jul 14 2021, 4:32 AM

@jroelofs I tried to run llvm/test/Transforms/Util/PredicateInfo/testandor.ll for both rG77080a1eb6061df2dcfae8ac84b85ad4d1e02031 (the moment of the commit) and rGfb9c5c3dce27b352534641dbb6e3cb8c05da7bc9 (recent main), with -DLLVM_ENABLE_EXPENSIVE_CHECKS=1.
I checked that the -DEXPENSIVE_CHECKS is added to the ninja.build file. But, I am not able to observe the issue you are mentioning. How can I reproduce this problem ?

Are you using the legacy pass manager? From the message Jon shared, it looks like it comes from the legacy PM.

Are you using the legacy pass manager? From the message Jon shared, it looks like it comes from the legacy PM.

That did the trick. @dberlin Created this pass, but I am not sure he is still active. Although this pass is used to dump information, it does seem to introduce llvm.ssa.copy calls. I'll see how to modify the return value so that it correctly identifies changes.