[PATCH] Clarify that the bypassSlowDivision optimization operates on a single BB.

[Justin Lebar via llvm-commits]

Hi, thank you for the feedback, Hal.

I tried making this change, which I agree would be a big readability
improvement.  But looking more closely at the code, I see that
bypassSlowDivision is modifying the Function::iterator it's passed.  I
think it's doing this to avoid running the bypassSlowDivision
optimization on the BBs added by bypassSlowDivision itself.

It's not clear to me what's the right way to fix this.  We could make
bypassSlowDivision operate over a whole Function, but that feels kind
of wrong, since bypassSlowDivision can sanely be applied to a single
BB.

Definitely if we leave things as-is, the comment on bypassSlowDivision
should be updated to indicate how it modifies the Function::iterator
it's passed.

Thank you again for your help and patience.

-Justin


On Wed, Dec 9, 2015 at 8:55 PM, Hal Finkel <hfinkel at anl.gov> wrote:
> Hi Justin,
>
> This is better. Our general convention is to use BasicBlock * and Instruction * on interface boundaries. I think using those instead of the iterator types would make things even clearer (and more consistent with our general conventions).
>
>  -Hal
>
> ----- Original Message -----
>> From: "Justin Lebar" <jlebar at google.com>
>> To: "Hal Finkel" <hfinkel at anl.gov>, "Tyler Nowicki" <tyler.nowicki at gmail.com>
>> Sent: Wednesday, December 9, 2015 4:36:25 PM
>> Subject: [PATCH] Clarify that the bypassSlowDivision optimization operates on a single BB.
>>
>> Remove the Function& arg to bypassSlowDivision -- it isn't necessary,
>> and makes it look like this function operates on a whole function
>> rather
>> than on one basic block.
>>
>> Also update some comments to be more explicit.
>> ---
>>  include/llvm/Transforms/Utils/BypassSlowDivision.h |  8 ++--
>>  lib/CodeGen/CodeGenPrepare.cpp                     |  2 +-
>>  lib/Transforms/Utils/BypassSlowDivision.cpp        | 53
>>  ++++++++++------------
>>  3 files changed, 29 insertions(+), 34 deletions(-)
>>
>
> --
> Hal Finkel
> Assistant Computational Scientist
> Leadership Computing Facility
> Argonne National Laboratory