On Wed, 12 Oct 2016, Richard Biener wrote:
So with doing the same on GENERIC I hit
FAIL: g++.dg/cpp1y/constexpr-array4.C -std=c++14 (test for excess errors)
with the pattern
/* (T)(P + A) - (T)P -> (T) A */
(for add (plus pointer_plus)
(simplify
(minus (convert (add @0 @1))
(convert @0))
Ah, grep missed that one because it is on 2 lines :-(
...
(convert @1))))
no longer applying to
(long int) ((int *) &ar + 12) - (long int) &ar
because while (int *) &ar is equal to (long int) &ar (operand_equal_p
does STRIP_NOPS) they obviously do not have the same type.
I believe we are comparing (int *) &ar to &ar, not to (long int) &ar. But
yes, indeed.
So on GENERIC we have to consider that we feed operand_equal_p with
non-ATOMs (arbitrary expressions). The pattern above is "safe" as it
doesn't reference @0 in the result (not sure if we should take advantage
of that in genmatch).
Since we are in the process of defining an
operand_equal_for_(generic|gimple)_match_p, we could tweak it to check the
type only for INTEGER_CST, or to still STRIP_NOPS, or similar.
Or we could remain very strict and refine the pattern, allowing a convert
on the pointer (we might want to split the plus and pointer_plus versions
then, for clarity). There are not many optimizations that are mandated by
front-ends and for which this is an issue.
The other FAILs with doing the same on GENERIC are
FAIL: g++.dg/gomp/declare-simd-3.C -std=gnu++11 (test for excess errors)
FAIL: g++.dg/torture/pr71448.C -O0 (test for excess errors)
FAIL: g++.dg/vect/simd-clone-6.cc -std=c++11 (test for excess errors)
the simd ones are 'warning: ignoring large linear step' and the pr71448.C
case is very similar to the above.
Yes, I expect they all come from the same 1 or 2 transformations.
If we stick to the old behavior, maybe we could have some genmatch
magic to help with the constant capture weirdness. With matching
captures, we could select which operand (among those supposed to be
equivalent) is actually captured more cleverly, either with an
explicit marker, or by giving priority to the one that is not
immediatly below convert? in the pattern.
This route is a difficult one to take
The simplest version I was thinking about was @0 for a true capture, and @@0
for something that just has to be checked for equality with @0.
Hmm, ok. So you'd have @@0 having to match @0 and we'd get the @0 for
the result in a reliable way? Sounds like a reasonable idea, I'll see
how that works out (we could auto-detect '@@' if the capture is not
used in the result, see above).
It probably doesn't bring much compared to the @0@4 syntax you were
suggesting below, so if that is easier...
-- what would be possible is to
capture a specific operand. Like allow one to write
(op (op @0@4 @1) (op @0@3 @2))
and thus actually have three "names" for @0. We have this internally
already when you write
(convert?@0 @1)
for the case where there is no conversion. @0 and @1 are the same
in this case.
Looks nice and convenient (assuming lax constant matching).
Yes, w/o lax matching it has of course little value.
Not sure if this helps enough cases.
IIRC, in all cases where we had trouble with operand_equal_p, chosing which
operand to capture would have solved the issue.
Yes. We'd still need to actually catch all those cases...
Being forced to chose which operand we capture (say with @ vs @@, making 2
occurences of @0 a syntax error) might help, but it would also require
updating many patterns for which this was never an issue (easy but
boring).
I still think that having two things matched that are not really
the same is werid (and a possible source of errors as in, ICEs,
rather than missed optimizations).
Ok. Let's go with the strict matching, it is indeed less confusing.
I'll hold off with the GENERIC strict matching and will investigate
the @@ idea (plus auto-detecting it).
And if we move to stricter matching, maybe genmatch could be updated so
convert could also match integer constants in some cases.
You mean when trying to match @0 ... (convert @0) and @0 is an INTEGER_CST
allow then (convert ...) case to match an INTEGER_CST of different type?
That's an interesting idea (not sure how to implement that though).
Yes, though I am not sure of the exact semantics that would work best.
On a related note, seeing duplicated patterns for constants, I tried several
variants of
(match (mynot @0)
(bit_not @0))
(match (mynot @0)
INTEGER_CST@0
(if (@0 = wide_int_to_tree (TREE_TYPE (@0), wi::bit_not (@0)))))
(simplify
(minus (bit_and:cs @0 (mynot @1)) (bit_and:cs @0 @1))
(minus (bit_xor @0 @1) @1))
This kind of hack feels wrong, but I don't see a proper way to write it.
Yes. The above is very much a hack. Must have been me inventing it
just to avoid duplicating the pattern.
Uh, no, don't worry, we don't have that hack in match.pd, we have 2
patterns. The hack is just me trying to remove the duplication. If you
thought you had written it, that means it may not have been such an absurd
way to go about it ;-)
--
Marc Glisse
