Page MenuHomePhabricator

[ADCE] Refactoring for new functionality (NFC)

Authored by david2050 on Aug 2 2016, 9:44 PM.



This is another refactoring to break up the one function into three logical components functions.
Another non-functional change before we start added in features.

Diff Detail

Event Timeline

david2050 updated this revision to Diff 66612.Aug 2 2016, 9:44 PM
david2050 retitled this revision from to [ADCE] Refactoring for new functionality (NFC).
david2050 updated this object.
david2050 added reviewers: nadav, mehdi_amini, majnemer.
david2050 added subscribers: llvm-commits, freik.
nadav added inline comments.Aug 3 2016, 10:10 AM

Please use doxygen style comments (three slashes) and a period at the end of the sentence. Please do the same for the other comments above.

Also, please name the parameter. If the name does not give a good indication of what the parameter is you can use '\p' to describe the parameter.,


Doxygen, period, just like above.


Typo here. 'gig'

david2050 updated this revision to Diff 66676.Aug 3 2016, 10:25 AM

Doxygen style comments with periods. Fix spelling of Aggressive

twoh added a subscriber: twoh.Aug 3 2016, 10:27 AM
mehdi_amini added inline comments.Aug 3 2016, 10:29 AM

Also, just a nit, I'm used to to read Return true for... instead of True for.

majnemer added inline comments.Aug 3 2016, 10:30 AM

Would it make sense to use !isInstructionTriviallyDead here instead?


Perhaps the logic for profile instrumentation can be sunk into isInstructionTriviallyDead?

david2050 updated this revision to Diff 66679.Aug 3 2016, 10:50 AM

Save David's suggestions. Add parameter name

david2050 added inline comments.Aug 5 2016, 10:04 AM

I tried to reply via email but it got lost in space. I prefer to not make this change until I get the my changes landed just for stability of baseline. But I will get to to it before we are done.

majnemer accepted this revision.Aug 5 2016, 10:14 AM
majnemer edited edge metadata.



Sure, sounds reasonable to me.

This revision is now accepted and ready to land.Aug 5 2016, 10:14 AM
david2050 closed this revision.Aug 5 2016, 12:45 PM