On Mon, Aug 7, 2017 at 11:54 PM, Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> wrote: > On Mon, Aug 7, 2017 at 12:12 PM, Luc Van Oostenryck > <luc.vanoostenryck@xxxxxxxxx> wrote: >> + // replace (A << S) >> S >> + // by A & (-1 >> S) > > Umm. This seems misleading. > > Don't you mean "value of all ones of the same size as A, shifted right by S" > >> + insn->src2 = value_pseudo(-1ULL >> value); > > ..and this seems buggy. There's a lot of bits in -1ULL, but what if > the size of 'S' is only 32-bit? > > So I think you should take the size of A into account? > > Or am I missing something? I don't think you're missing something and this code have only been very lightly tested (and maybe developed too fastly too). OTOH, it shouldn't matter much since it's only used for a mask (subsequent steps will drop the superfluous high bits) and I think that it's done so at some other place too. But it certainly wouldn't be bad to use the right size for the mask. -- Luc -- To unsubscribe from this list: send the line "unsubscribe linux-sparse" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
