On Thu, Aug 09, 2018 at 12:05:56PM -0700, Linus Torvalds wrote:
> On Thu, Aug 9, 2018 at 2:20 AM Luc Van Oostenryck
> <luc.vanoostenryck@xxxxxxxxx> wrote:
> >
> > > if (nmask == 0)
> > > return replace_pseudo(insn, &insn->src1, other);
> > > - return 0;
> >
> > There is a
> > if (nbr_users(insn->src1) > 1)
> > return 0;
> > missing here.
>
> Doin't you mean
>
> if (multi_users(insn->src1))
> return 0;
>
> in the new world order?
Well, yes and no.
Because it's the last part of an old branch (and I often avoid to
rebase in this case), it's still in the old world nbr_users().
I'll either rebase it later or add a path doing the conversion
once it's merged with the master branch.
> Side note, and independently of that comment, and the real reason I'm
> asking: it would really be lovely for all these "simplify" commits to
> have example code generation changes.
>
> I'm betting you have some simple bitfield code examples that you are
> using to guide these things, wouldn't it be nice to have those
> examples as part of the explanation for why you do particular
> simplifications?
Yes, absolutely.
Months ago I did this more often but now I tend to focus more on
the tests (so often now, a series begin with a testcase with the
goal to reach and set as known-to-fail, then at some stages the
known-to-fail can be removed). I also try to exaplain a bit the
simplification as a comment in the code itself. But yes adding an
example in the description would be very good.
Now, it seems that yesterday, I must have been tired because I made
this error here, I didn't add the testcase(s) and I forgot the signoffs.
> I realize that this isn't what has been normally done, and it
> obviously depends on the particular simplification. Some
> simplifications are so obvious that they don't really even merit an
> example. But this one strikes me as pretty involved from the
> description, even though I suspect that the actual bitfield operation
> that triggers this is very simple.
Yes, indeed. In this case, I just would like to simplify storing
a bitfield and reading it back. For example:
struct s {
unsigned int :2;
unsigned int f:3;
};
int foo(struct s s, int a)
{
s.f = a;
return s.f;
}
which now give:
foo:
and.32 %r11 <- %arg2, $7
ret.32 %r11
instead of:
foo:
and.32 %r4 <- %arg2, $7
shl.32 %r5 <- %r4, $2
and.32 %r6 <- %arg1, $0xffffffe3
or.32 %r7 <- %r6, %r5
lsr.32 %r9 <- %r7, $2
and.32 %r11 <- %r9, $7
ret.32 %r11
I'll add all this in the patch description.
This is in fact the last step before I integrate the SSA rewrite
(which I hope to do real soon now, next week I hope) because there
was some regression there with the introduction of PSEUDO_UNDEFs
(for the tracking of undefined pseudo which, among other things,
allows to avoid loops of self-defined pseudos, what's I call
"cyclic 'DAGs'" and is blocking a lot of simplifications).
I must say that I'm struggling a bit lately with the simplification
of bitfield accesses and I'm more and more inclined to add bitfield
insert & bit field extract instruction. It would help a lot but it
would also force to duplicate or redo a lot of the simplifications
already done for ASR/LSR/SH/AND/OR.
-- Luc
