On 03/10/2018 16:59, Alexei Starovoitov wrote:

On Wed, Oct 03, 2018 at 04:36:31PM +0100, Jiong Wang wrote:

<snip...>

quoted

Now this hasn't happened. I am still debugging the root cause, but kind of
feel
"64-bit" attribute propagation is the issue, it seems to me it can't be
nicely
integrated into the existing register read/write propagation infrastructure.

may be share your patches that modify the liveness propagation?

OK, I will share it after some clean up.

quoted

For
example, for a slightly more complex sequence which is composed of three
states:

State A
  ...
  10: r6 = *(u32 *)(r10 - 4)
  11: r7 = *(u32 *)(r10 - 8)
  12: *(u64 *)(r10 - 16) = r6
  13: *(u64 *)(r10 - 24) = r7

State B
  14: r6 += 1
  15: r7 += r6
  16: *(u32 *)(r10 - 28) = r7

State C
  ...
  17: r3 += r7
  18: r4 = 1
  19: *(u64 *)(r10 - 32) = r3
  20: *(u64 *)(r10 - 40) = r4

State A is parent of state B which is parent of state C.

Inside state C, at insn 20, r4 is a 64-bit read/use, so its define at 18 is
marked as "64-bit". There is no register source at 18, so "64-bit" attribute
propagation is stopped.

Then at insn 19, r3 is a 64-bit read/use, so its define at 17 is marked as
"64-bit" read/use. Insn 17 has two register sources, r3 and r7, they become
"64-bit" now, and their definition should be marked as "64-bit".

Now if the definition of r3 or r7 comes from parent state, then the parent

... the definition of r3 _and_ r7 ...
both need to propagate up with your algo, right?

Yes, all sources of insn 17, both r3 and r7.

quoted

state
should receive a "REG_LIVE_READ64", this is necessary if later another path
reaches state C and triggers prune path, for which case that path should
know
there is "64-bit" use inside state C on some registers, and should use this
information to mark "64-bit" insn.

If the definition of r3 or r7 is still inside state C, we need to keep
walking
up the instruction sequences, and propagate "64-bit" attribute upward until
it
goes beyond the state C.

The above propagation logic is quite different from existing register
read/write propagation.
For the latter, a write just screen up all following read, and a
read would propagate directly to its parent is there is not previous write,
no instruction analysis is required.

correct.
with such algo REG_LIVE_WRITTEN shouldn't be screening the propagation.

I think the patches will discuss the algo.
Also I think the initial state of 'everything is 32-bit safe'
and make marks to enforce 64-bit-ness is a dangerous algorithmic choice.

Indeed, and I am actually thinking the same thing ...

Why not to start at a safer state where everything is 64-bit
and work backward to find out which ones can be 32-bit?
That will be safer algo in case there are issues with bit like
you described in above.

... but I failed to find a algo works on making initial state of
"everything is 64-bit". I haven't figured out a way to check that all use of one
definition are 32-bit on all possible code paths, which looks to me is a must for
one insn marked as 32-bit safe.