Re: [PATCH] middle end, s390: optimization for builtin isnan

[Wolfgang Gellerich <gellerich at de dot ibm dot com>]

> Sorry, you're right, my suggestion doesn't make sense.  Still, I'm not
> that fond of target-specific target hooks.  Also, it's not obvious to
> me that your suggestion is always an improvement.  It's clearly an
> improvement for your test case, but what happens when there is no
> floating point operation to set the condition code?  As in
>     foo (double x) { return isnan (x); }
> Don't you wind up materializing a floating point zero in order to do
> the comparison, thus adding an unnecessary instruction?

Yes, a floating point zero can be much more expensive than
the value suggests-:) However, on s390, comparing a register with float
const zero is implemented by executing a load-and-test instruction
storing the register into itself. The instruction has the side effect to
set the conditon code as if the value being loaded
had been compared with zero and this is exploitet to avoid having to
load the zero explicitly. Source code like the example you
provided actually compiles into code containing load-and-test in a gcc
with the patch applied. To summarize, in all cases where the patch
does not eliminate the compare at all it replaces the compare by a load.

> Looking at your e-mail more closely, I expect that what you want can
> be handle with a define_split which runs at combine time.  You want
> the split to recognize the three instructions
>     adbr f2, f3
>     cdbr f2, f2
>     jo label
> and combine them into simply
>     adbr f2, f3
>     jo label
> This combination of three instructions into two is what combine-time
> define_splits do.

Yes, this is the effect I want to achieve. The problem is that there is a
large number of  instructions that set the condition code the right way:
most if not all arithmetic operations, some of the load instructions (as
mentioned above) and so on. This is good because the compare
instruction previously generated for the isnan will be omitted in many
cases. On the other hand, every of these instructions already has
several (!) insns in s390.md to describe their main purpose, their side
effect(s) of setting the condition code, and combinations thereof.
Adding even more variants to catch the case of one register getting
compared with itself would cause a significant amount of new descriptions.

Also, combine already has some special code to recognize those cases
where a compare with zero cen be eliminated because a previously
executed instruction has already set the condition code that way.
Probably, additional support were needed in combine to catch the cases
where an operand is not compared to zero but with itself.

Therefore, my suggestion is to re-use the existing for optimizations based
on other instructions setting the compared-with-zero condition code
which is already there in combine and in some back ends by changing
the isnan implementation from a compare of the argument with itself to
a compare against zero. This is just a one line change. The remainder
of the patch is only there to make this change optional via the target
hook.

Regards, Wolfgang