On 05/22/2015 09:42 AM, Alan Lawrence wrote:
This patch does so (and makes slightly less conservative, to tackle the
example above). I found I had to make this a separate pass, so that the
phi nodes were cleaned up at the end of the pass before running
tree_if_conversion. Also at this stage in the compiler (inside loop
opts) it was not possible to run loop_optimizer_init+finalize, or other
loop_optimizer data structures needed later would be deleted; hence, I
have two nearly-but-not-quite-identical passes, the new "ch_vect"
avoiding the init/finalize. I tried to tackle this with some C++
subclassing, which removes the duplication, but the result feels a
little ugly; suggestions for any neater approach welcome.
What PHI node cleanup needs to be done? I don't doubt something's
needed, but would like to understand the cleanup -- depending on what
needs to be done, it may be the case that we can cleanup on-the-fly or
it may point at a general issue we should be resolving prior to running
tree_if_conversion.
This patch causes failure of the scan-tree-dump of dom2 in
gcc.dg/ssa/pr21417.c. This looks for jump-threading to perform an
optimization, but no longer finds the expected line in the log - as the
loop-header-copying phase has already done an equivalent transformation
*before* dom2. The final CFG is thus in the desired form, but I'm not
sure how to determine this (scanning the CFG itself is very difficult,
well beyond what we can do with regex, requiring looking at multiple
lines and basic blocks). Can anyone advise? [The test issue can be
worked around by preserving the old do_while_p logic for the first
header-copying pass, and using the new logic only for the second, but
this is more awkward inside the compiler, which feels wrong.]
Don't we have a flag to turn off loop header copying? If so, does
adding that flag to the test "fix" it without resorting to something
gross like preserving the old logic for the first pass and new logic for
the second pass.
The refactoring to deal with being able to call into this without
reinitializing the loop optimizer doesn't seem terrible to me. One
could argue that the loop optimizer init bits could become a property
and managed by the pass manager. I'm not sure that really simplifies
anything though.
My biggest worry would be cases where data initialized by
loop_optimizer_init gets invalidated by the header copying. Have you
looked at all at that possibility? I don't have anything specific in
mind to point you at -- just a general concern.
Besides the new vect-ifcvt-11.c, the testsuite actually has a couple of
other examples where this patch enables (undesired!) vectorization. I've
dealt with these, but for the record:
Presumably undesired is within the scope of the testsuite, not
necessarily in terms of the code we generate for real user code :-)
Overall it doesn't look bad to me... Convince me it's safe WRT the
loop_optimizer_init issue above and we'll have a clear path forward.
jeff
