[llvm] r240521 - Hexagon: Avoid left shifting negative values (it's UB)

[Justin Bogner]

David Majnemer <david.majnemer at gmail.com> writes:
> On Tue, Jun 23, 2015 at 11:00 PM, Justin Bogner <mail at justinbogner.com> wrote:
>
>     Author: bogner
>     Date: Wed Jun 24 01:00:53 2015
>     New Revision: 240521
>    
>     URL: http://llvm.org/viewvc/llvm-project?rev=240521&view=rev
>     Log:
>     Hexagon: Avoid left shifting negative values (it's UB)
>    
>     Found by ubsan.
>    
>     Modified:
>         llvm/trunk/lib/Target/Hexagon/MCTargetDesc/HexagonShuffler.h
>    
>     Modified: llvm/trunk/lib/Target/Hexagon/MCTargetDesc/HexagonShuffler.h
>     URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/Hexagon/
>     MCTargetDesc/HexagonShuffler.h?rev=240521&r1=240520&r2=240521&view=diff
>     ==========================================================================
>     ====
>     --- llvm/trunk/lib/Target/Hexagon/MCTargetDesc/HexagonShuffler.h
>     (original)
>     +++ llvm/trunk/lib/Target/Hexagon/MCTargetDesc/HexagonShuffler.h Wed Jun
>     24 01:00:53 2015
>     @@ -34,7 +34,7 @@ public:
>        HexagonResource(unsigned s) { setUnits(s); };
>    
>        void setUnits(unsigned s) {
>     -    Slots = s & ~(-1 << HEXAGON_PACKET_SIZE);
>     +    Slots = s & ~(~0U << HEXAGON_PACKET_SIZE);
>
> ISTM that this is equivalent to `s & ((1 << HEXAGON_PACKET_SIZE) - 1)`. 
> Personally, I find that this version is more obvious while still avoiding UB.

They are equivalent. I don't find the version with the subtraction
particularly more obvious. In one case you do NOT twice, which is
confusing, but you only have to deal in flipping bits, in the other you
have to think about arithmetic's effects on a bit mask. They're both a
bit awkward, but also fairly obvious.

>
>          setWeight(s);
>        };
>        unsigned setWeight(unsigned s);
>
>     _______________________________________________
>     llvm-commits mailing list
>     llvm-commits at cs.uiuc.edu
>     http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits