On Sat, Jul 28, 2018 at 03:29:52AM +0100, Ramsay Jones wrote: > > > On 28/07/18 02:20, Ramsay Jones wrote: > [snip] > >> + // setne.1 %r <- %s, $0 > >> + // into: > >> + // setne.1 %s <- %a, $0 > >> + // and same for setne/eq ... 0/1 > >> + return replace_pseudo(insn, &insn->src1, def->src); > > > > Hmm, so this isn't just removing the conditional. In the context > > I can see that the last argument to replace_pseudo was def->src1 > > rather than def->src used here. I haven't given it much thought, > > just reading the patch text, but ... > > OK, so I already knew that the pseudo_t's src and src1 were > effectively aliases (since they are stored at the same position > in the union), but they shouldn't be used interchangeably. > > The difference is the 'arrity' of the instructions; unary > versus binary. So, this is actually a 'fix' (zext is a unary > operator)? I see it more as an automatic thing than a fix. In the doc (IR.rst) I've documented that src, src1, cond and some others are all aliased but I try to keep things consistent with the declarations: 'src' for unops, src1 for binops, ... The '*ext' is there because I splitted the patch when I begin to write the patch description and I forgot to change what was once correctly '*ext' into 'zext'. Thanks for noticing. -- 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