Re: [PATCH, rs6000] Use CC for BCD operations [PR100736]

[Segher Boessenkool <segher@kernel.crashing.org>]

Hi!

On Thu, Jun 16, 2022 at 02:36:53PM +0800, HAO CHEN GUI wrote:
>   This patch uses CC instead of CCFP for all BCD operations. Thus, infinite
> math flag has no impact on BCD operations. To support BCD overflow and
> invalid coding, ordered and unordered are added into CC mode.

This is wrong.  Unordered is an FP concept.  What the BCD insns do is
write to bit 3 in a CR field, which for FP comparisons is the
"unordered" comparison result (FP comparisons result in one of four
options: lt, gt, eq, or un).  For integer comparisons there are only
three possible comparison results (lt, gt, and eq), and bit 3 is set to
a copy of XER[SO], the "summary overflow" bit.

The BCD insns have the one-out-of-three comparison result as well, and
they set bit 3 if overflow happened.  There is the extra gotcha that it
sets the four condition field bits to 0001 if there is an invalidly
coded input, but we can ignore that: it is not supposed to happen.

There is no normal way to get at bit 3 of a CR field.  We can use some
unspec though, which is total cheating but it does work, and it is
safe, albeit sometimes suboptimal.

> With CC,
> "ge" and "le" are converted to reverse comparison. So the invalid coding
> needs to be tested separately.

Bit 3 has no meaning in integer comparisons, in GCC, it is not modeled.
We handle this with some unspec usually.  We need to for vector insns
that set a CR field for example: they can set more than one bit to 1, or
all bits to 0, neither of which is valid in any MODE_CC we have.

We should add proper CC modes for all the ways that instructions can set
the CR field bits.  But this is much less trivial than it may seem, so
I'd like the PR to be fixed in a simple way first (a way that can be
backported, too!)

> 	* config/rs6000/altivec.md (bcd<bcd_add_sub>_<mode>): Replace CCFP with
> 	CC

Sentences end in a full stop.  I'd say
	....................................................: Replace CCFP
	with CC.
so that it doesn't look as silly / strange :-)

> +  rtx cr6 = gen_rtx_REG (CCmode, CR6_REGNO);
> +  rtx condition_rtx = gen_rtx_<CODE> (SImode, cr6, const0_rtx);
> +  rtx_code cond_code = GET_CODE (condition_rtx);

That GET_CODE always would return <CODE>, fwiw.

You shouldn't need anything like this, bcdinvalid will work just fine if
written as bcdadd_ov (with vector of 0 as the second op)?


Segher