Page MenuHomePhabricator

Add optimization bisect opt-in calls for PowerPC passes
ClosedPublic

Authored by andrew.w.kaylor on Apr 26 2016, 1:00 PM.

Details

Summary

This patch adds calls to PowerPC-specific passes that can be safely skipped to opt-in to the optimization bisect mechanism.

I selected the passes to be skipped based on the fact that they were not added at CodeGenOpt::None in PPCTargetMachine.cpp. Based on that criteria, I did not add opt-in calls to the following passes, which appear to be required:

PPCBSel
PPCTOCRegDeps
PPCTLSDynamicCall
PPCVSXCopy
PPCVSXFMAMutate

I also chose not to add the skip check to PPCCTRLoopsVerify, but I'm not certain about this one. This pass isn't run at -O0, so it meets the normal criteria for skipping, but since it is a verification function I decided not to add the skip check. If this is verifying that transformations were performed in PPCCTRLoops, which can be skipped, then there is a potential problem. If, on the other hand, it is simply verifying that the IR is legal following PPCCTRLoops then it should be fine to not skip it.

Note that the call to skipFunction() will also check for the "optnone" function attribute, so this can theoretically result in passes being skipped even when optimization bisect is not being done. However, I believe that any pass that can be safely skipped should be skipped for functions with the optnone attribute.

See D19172 for details on the base optimizaton bisect implementation.

Diff Detail

Repository
rL LLVM

Event Timeline

andrew.w.kaylor retitled this revision from to Add optimization bisect opt-in calls for PowerPC passes.
andrew.w.kaylor updated this object.
andrew.w.kaylor added a reviewer: hfinkel.
andrew.w.kaylor set the repository for this revision to rL LLVM.
andrew.w.kaylor added a subscriber: llvm-commits.
hfinkel edited edge metadata.Apr 26 2016, 1:59 PM

I selected the passes to be skipped based on the fact that they were not added at CodeGenOpt::None in PPCTargetMachine.cpp. Based on that criteria, I did not add opt-in calls to the following passes, which appear to be required:

PPCBSel
PPCTOCRegDeps
PPCTLSDynamicCall
PPCVSXCopy
PPCVSXFMAMutate

PPCVSXFMAMutate is an optimization, and should be included in the skippable passes.

I also chose not to add the skip check to PPCCTRLoopsVerify, but I'm not certain about this one. This pass isn't run at -O0, so it meets the normal criteria for skipping, but since it is a verification function I decided not to add the skip check. If this is verifying that transformations were performed in PPCCTRLoops, which can be skipped, then there is a potential problem. If, on the other hand, it is simply verifying that the IR is legal following PPCCTRLoops then it should be fine to not skip it.

PPCCTRLoopsVerify is verifying work that PPCCTRLoops performed, and can only be skipped whenever PPCCTRLoops is skipped.

PPCVSXFMAMutate is an optimization, and should be included in the skippable passes.

It looks to me as if PPCVSXFMAMutate is always added (in PPCPassConfig::addPreRegAlloc), even at -O0. I'm trying to keep the "optnone" function handling and the bisection consistent with -O0. There's a lit test that verifies that nothing is skipped because of the "optnone" function attribute that is otherwise run at -O0. I plan to add a similar check for the opt bisect soon.

It makes sense that PPCVSXFMAMutate would be skippable, but I'd prefer not to add it to the bisection unless it is also being omitted at -O0.

PPCCTRLoopsVerify is verifying work that PPCCTRLoops performed, and can only be skipped whenever PPCCTRLoops is skipped.

If I add the skip check to PPCCTRLoopsVerify, it will always be skipped if PPCCTRLoops is skipped (because it runs afterward), but it's possible that PPCCTRLoops could be run and PPCCTRLoopsVerify. That should be OK, though right? So should I add the skip check to the verify pass?

PPCVSXFMAMutate is an optimization, and should be included in the skippable passes.

It looks to me as if PPCVSXFMAMutate is always added (in PPCPassConfig::addPreRegAlloc), even at -O0. I'm trying to keep the "optnone" function handling and the bisection consistent with -O0. There's a lit test that verifies that nothing is skipped because of the "optnone" function attribute that is otherwise run at -O0. I plan to add a similar check for the opt bisect soon.

It makes sense that PPCVSXFMAMutate would be skippable, but I'd prefer not to add it to the bisection unless it is also being omitted at -O0.

Okay; in that case, please change that as well. It can be skipped at -O0 too.

PPCCTRLoopsVerify is verifying work that PPCCTRLoops performed, and can only be skipped whenever PPCCTRLoops is skipped.

If I add the skip check to PPCCTRLoopsVerify, it will always be skipped if PPCCTRLoops is skipped (because it runs afterward), but it's possible that PPCCTRLoops could be run and PPCCTRLoopsVerify. That should be OK, though right? So should I add the skip check to the verify pass?

No, don't skip it.

andrew.w.kaylor edited edge metadata.

Added code to check of opt level != None before adding the PPCVSXFMAMutate pass
Made the PPCVSXFMAMutate pass skippable

hfinkel accepted this revision.Apr 26 2016, 6:05 PM
hfinkel edited edge metadata.

LGTM.

This revision is now accepted and ready to land.Apr 26 2016, 6:05 PM
This revision was automatically updated to reflect the committed changes.