[PATCH] D21645: [CFLAA] Propagate StratifiedAttrs from callee to caller

George Burgess IV via llvm-commits llvm-commits at lists.llvm.org
Mon Jun 27 11:45:14 PDT 2016


Fixed in r273902. Thanks for pinging this!

(Also, in the future, if you could add reviewers/the patch author to the CC
line for things like this, that would be really helpful. Many of us have
lots of email filters set up, so being cc'ed makes a difference. :) )

On Sat, Jun 25, 2016 at 12:43 PM, Hans-Bernhard Bröker <
llvm-commits at lists.llvm.org> wrote:

> Hello,
>
> and please excuse my barging in like this.  I think one detail of the
> recent patch r273636 is wrong.  The shift operation in
>
> static StratifiedAttrs argNumberToAttr(unsigned ArgNum) {
>   if (ArgNum >= AttrMaxNumArgs)
>     return AttrUnknown;
>   return StratifiedAttrs(1 << (ArgNum + AttrFirstArgIndex));
> }
>
> is fishy, and MSVC rightly warns about it:
>
>  [...]\llvm\lib\Analysis\CFLAliasAnalysis.cpp(765): warning C4334: '<<':
> result of 32-bit shift implicitly converted to 64 bits (was 64-bit shift
> intended?)
>
> The problem is that the literal '1', and thus the result of the shift, is
> a plain signed int, whereas the constructor of StatifiedAttrs() takes an
> argument of unsigned long long.  On 64-bit Windows, these are 32-bit signed
> and 64-bit unsigned, respectively, hence the warning.
>
> Shifts like that should always be done entirely in the realm of unsigned
> numbers, i.e. that should be `1U' instead of `1'.  And if the result is
> going to end up being implicitly converted to long long, it's best to write
> it as that directly:
>
>   return StratifiedAttrs(1ULL << (ArgNum + AttrFirstArgIndex));
>
> Hans-Bernhard Bröker
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160627/6fcb8af2/attachment.html>


More information about the llvm-commits mailing list