On Sun, Nov 22, 2020 at 11:00:26AM -0800, Linus Torvalds wrote:
> I thought we already canonicalized the pseudo ordering, but clearly not..
>
> Anyway., looks good to me.
>
> Btw, if you want to do another simplification, one that I've actually
> seen multiple times in the kernel is this one:
>
> if (val1 & BITx)
> val2 |= BITy;
>
> and turning it into
>
> val2 |= (val1 & BITx) .. shift left or right by (BITx-BITy);
Mmmm, yes, interesting. I'll look at this but ...
> and while actually testing the above, I note that sparse seems to have
> problems with even simple if-conversion:
>
> #define BIT1 4
> #define BIT2 8
>
> int fn(int x, int y)
> {
> if (x & BIT1)
> y |= BIT2;
> return y;
> }
>
> linearizes to a nasty mess of actual phi nodes and conditional jumps
> rather than just a 'select' op. Never mind the actual unconditional
> version, of course.
>
> I didn't check why the if-conversion doesn't happen.
Yes, I know. I'm currently working on it the last days but it's nasty.
Very often, a (good) change blocks another one and I'm often bitten by
the CFG changing and thus invalidating things like dominance relation-
ships.
In the case here with your example, the if-conversion doesn't happen
because the phi-sources is not defined in the top block because of
the OR:
fn:
and.32 %r2 <- %arg1, $4
phisrc.32 %phi2(y) <- %arg2
cbr %r2, .L1, .L2
.L1:
or.32 %r5 <- %arg2, $8
phisrc.32 %phi3(y) <- %r5
br .L2
.L2:
phi.32 %r8(y) <- %phi2(y), %phi3(y)
ret.32 %r8(y)
It's quite frequent and the obvious solution is to move up such
instructions (and maybe, if the if-conversion succeed, the BBs
will be merged and there won't be a top and bottom block anymore).
I've a very crude patch moving all such instructions up if possible,
it gives some positive results for the context imbalance but nothing
miraculous (IIRC something like 35 warnings less on a total of ~1000).
Another interesting example is something like:
int foo(int a)
{
int e;
if (a) {
__context__(1);
e = 1;
} else {
e = 0;
}
if (!e)
return 1;
__context__(-1);
return 0;
}
which produces the following IR:
foo:
.L0:
cbr %arg1, .L1, .L3
.L1:
context 1
br .L3
.L3:
seteq.32 %r5 <- %arg1, $0
cbr %arg1, .L5, .L6
.L5:
context -1
br .L6
.L6:
ret.32 %r5
Sparse issues a "context imbalance" warnings on it but in fact
everything is OK. Three different things could unblock things:
1) the seteq move up to L0, then L1 can directly jump to L5
2) the seteq move down to L6, then again L1 can jump to L5
3) the context in L1 is changed into a conditional (a new instruction,
essentially a select but for contexts) it can then be moved up
to L0, L1 disappears and L3 can be merged with L0.
None of these are really hard but the problem, I'm sure you can guess
it, is that none bring a definitive, clear solution because, very often,
moving things up or down just displaces the problem to somewhere else.
But well, it's life.
-- Luc
