Page MenuHomePhabricator

bugpoint Enhancement.
ClosedPublic

Authored by DiamondLovesYou on Mar 23 2015, 10:33 AM.

Details

Summary

This patch adds two flags to bugpoint: "-replace-funcs-with-null" and "-disable-pass-list-reduction".

When "-replace-funcs-with-null" is specified, bugpoint will, instead of simply deleting function bodies, replace all uses of functions and then will delete functions completely from the test module, correctly handling aliasing and @llvm.used && @llvm.compiler.used. This part was conceived while trying to debug the PNaCl IR simplification passes, which don't allow undefined functions (ie no declarations).

With "-disable-pass-list-reduction", bugpoint won't try to reduce the set of passes causing the "crash". This is needed in cases where one is trying to debug an issue inside the PNaCl IR simplification passes which is causing an PNaCl ABI verification error, for example.

Diff Detail

Repository
rL LLVM

Event Timeline

DiamondLovesYou retitled this revision from to bugpoint Enhancement..
DiamondLovesYou updated this object.
DiamondLovesYou edited the test plan for this revision. (Show Details)
DiamondLovesYou added a reviewer: jfb.
DiamondLovesYou set the repository for this revision to rL LLVM.
DiamondLovesYou added a subscriber: Unknown Object (MLST).
jfb edited edge metadata.Mar 27 2015, 2:43 PM

Could you also add tests for this in test/BugPoint/?

tools/bugpoint/CrashDebugger.cpp
49 ↗(On Diff #22483)

disable-pass-list-reduction would be more idiomatic.

209 ↗(On Diff #22483)

Isn't this always false? At least InstrProfiling.cpp does cast<ConstantArray>(LLVMUsed->getInitializer()) so one of these is wrong :-)

216 ↗(On Diff #22483)

Using Value *V is clearer.

298 ↗(On Diff #22483)

Can you instead replace all uses of the function with Undef of that type before deleting the function?

DiamondLovesYou updated this object.
DiamondLovesYou edited edge metadata.

Fixed JF's comments.

tools/bugpoint/CrashDebugger.cpp
49 ↗(On Diff #22483)

You're right; done!

209 ↗(On Diff #22483)

No, because LLVM will replace an array with zeroinitializer when the last non-null value is replaced with null.

216 ↗(On Diff #22483)

Done.

298 ↗(On Diff #22483)

undef isn't considered a null value, meaning isNullValue would return false. It's not impossible to do, but I'm guessing it would change quite a bit of behavior and given that calling either null or undef is undefined behavior, I'd rather not mess with it post-mortem.

jfb accepted this revision.Apr 6 2015, 8:35 AM
jfb edited edge metadata.

lgtm, let's leave this open if there are any other comments.

This revision is now accepted and ready to land.Apr 6 2015, 8:35 AM
This revision was automatically updated to reflect the committed changes.