[PATCH] Rename PerBBDivCache to DivCache in BypassSlowDivision.

Justin Lebar via llvm-commits llvm-commits at lists.llvm.org
Wed Dec 9 09:47:43 PST 2015


Oh, I see.  We even iterate over the BBs in
CodeGenPrepare::runOnFunction, it's quite clear.

Since I managed to convince one other person that there was something
up here, I may send a patch to change the comments slightly, if that's
OK.

Sorry for the noise.

-Justin

On Wed, Dec 9, 2015 at 8:46 AM, Tyler Nowicki <tyler.nowicki at gmail.com> wrote:
> Hi Justin, Hal,
>
>
>
> It is a BB level cache. bypassSlowDivision() is executed once per
> BasicBlock. Notice that the DivCache is locally defined in
> bypassSlowDivision(). So the cache would be empty when %Ret2 is encountered.
>
>
>
> Tyler
>
>
> From: Hal Finkel
> Sent: December 9, 2015 7:16 AM
> To: Justin Lebar
> Cc: preston gurd;llvm-commits;Tyler Nowicki
> Subject: Re: [PATCH] Rename PerBBDivCache to DivCache in BypassSlowDivision.
>
>
>
>
>
> Hi Justin,
>
>
>
> You seem to be right, but the code does not seem to be setup correctly for a
> function-level cache. Specifically, it seems to be missing a dominance check
> before reusing the result of the previous division expansion. Thus, if we
> have:
>
>
>
> %Dividend = ...
>
> %Divisor = ...
>
>
>
> if (%whatever) {
>
>   %Ret = %Dividend / %Divisor;
>
> } else {
>
>   %Ret2 = %Dividend / %Divisor;
>
> }
>
>
>
> what prevents this code from "reusing" the expansion for %Ret upon
> encountering %Ret2?
>
>
>
> -Hal
>
>
>
> ----- Original Message -----
>
>> From: "Justin Lebar via llvm-commits" <llvm-commits at lists.llvm.org>
>
>> To: "llvm-commits" <llvm-commits at lists.llvm.org>
>
>> Cc: "preston gurd" <preston.gurd at intel.com>
>
>> Sent: Tuesday, December 8, 2015 12:12:22 PM
>
>> Subject: [PATCH] Rename PerBBDivCache to DivCache in BypassSlowDivision.
>
>>
>
>> This hashtable is used for the whole function, not just the BB.
>
>>
>
>> No functional changes.
>
>> ---
>
>>  lib/Transforms/Utils/BypassSlowDivision.cpp | 13 ++++++-------
>
>>  1 file changed, 6 insertions(+), 7 deletions(-)
>
>>
>
>> _______________________________________________
>
>> llvm-commits mailing list
>
>> llvm-commits at lists.llvm.org
>
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
>>
>
>
>
> --
>
> Hal Finkel
>
> Assistant Computational Scientist
>
> Leadership Computing Facility
>
> Argonne National Laboratory
>
>
>
>


More information about the llvm-commits mailing list