[PATCH] D103788: [InstCombine] Eliminate casts to optimize ctlz operation

Datta Nagraj via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sat Jun 12 01:24:04 PDT 2021


datta.nagraj added a comment.

In D103788#2813446 <https://reviews.llvm.org/D103788#2813446>, @spatel wrote:

> In D103788#2813373 <https://reviews.llvm.org/D103788#2813373>, @datta.nagraj wrote:
>
>> Hi @spatel Sir, doesn't https://alive2.llvm.org/ce/z/89_wJb prove that this works for even type smaller than log2 of the wide type ? I can add the same test to the unit test, but I am in doubt whether to add it as a check, since the example here proves that it works for difference of bitwidth of more than log 2 as well. I tested with 63, 2 sizes as well, and that works too. Please suggest if we require to add the log2 check, since these are working fine from the above examples.
>
> That test proves that the transform is correct for that pair of types exactly. It does not prove that the transform is correct for all pairs of types. If you can prove *why* truncating the addition constant will always work, then we can proceed without the additional check. If not, please add the extra check and mark the test with a 'TODO' comment that references what we have discussed here.
>
> I'd rather be safe than cause a miscompile on some pair of types that we did not think of (and then potentially have the whole patch reverted). :)

Done. Agree to all your points Sir. :)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D103788/new/

https://reviews.llvm.org/D103788



More information about the llvm-commits mailing list