On 6/29/24 06:44, Jeff Law wrote:
>> +;; fclass instruction output bitmap
>> +;;   0 negative infinity
>> +;;   1 negative normal number.
>> +;;   2 negative subnormal number.
>> +;;   3 -0
>> +;;   4 +0
>> +;;   5 positive subnormal number.
>> +;;   6 positive normal number.
>> +;;   7 positive infinity
>> +;;   8 signaling NaN.
>> +;;   9 quiet NaN
>> +(define_insn "fclass<ANYF:mode>"
>> +  [(set (match_operand:SI           0 "register_operand" "=r")
>> +    (unspec:SI [(match_operand:ANYF 1 "register_operand" " f")]
>> +               UNSPEC_FCLASS))]
>> +  "TARGET_HARD_FLOAT"
>> +  "fclass.<fmt>\t%0,%1"
>> +  [(set_attr "type" "fcmp")
>> +   (set_attr "mode" "<UNITMODE>")])
> So I realize the result only has 10 bits of output, but I think would it 
> make more sense to use X rather than SI for the result.  When we use 
> SImode on rv64 we have to deal with potential extensions.  In this case 
> we know the values are properly extended, so we could just claim it's 
> DImode and I think everything would "just work" and we wouldn't have to 
> worry about unnecessary sign extensions creeping in.

Indeed the perils of sign extension on RV are not lost on me and this is
exactly how I started.
But my md syntax foo/bar is, lets just say a work in progress :-)

I started with

+ "fclass<ANYF:mode><X:mode>" so its invocation became + emit_insn
(gen_fclass<ANYF:mode><X:mode> (tmp, operands[1])); which then led to
expander itself needing X in the definition lest we get duplicate
definitions due to X's variants.

+(define_expand "isnormal<ANYF:mode><X:mode>2"
+  [(set (match_operand:X               0 "register_operand" "=r")
+       (unspec:X [(match_operand:ANYF 1 "register_operand" " f")]
+                  UNSPEC_ISNORMAL))]
+  "TARGET_HARD_FLOAT"

But this was not getting recognized as a well known pattern:
CODE_FOR_isnormalxx was not getting generated.

Keeping it as following did make it work.

+(define_expand "isnormal<ANYF:mode>2"

Any ideas on how I can keep this and then adjust rest of patterns.

Would it help to make it define_insn *name ?

>> +;; TODO: isinf is a bit tricky as it require trimodal return
>> +;;  1 if 0x80, -1 if 0x1, 0 otherwise
> It shouldn't be terrible, but it's not trivial either.
>
> bext t0, a0, 0
> neg t0
> bext t1, a0, 7
> czero.nez res, t0, t1
> snez t1, t1
> add a0, a1, a0
>
> Or something reasonably close to that.

I wrote the "C" code and saw what compiler would do ;-) for the baseline
isa build.

    andi    a5,a0,128
    bne    a5,zero,.L7
    slli    a5,a0,63
    srai    a0,a5,63
    ret
.L7:
    li    a0,1
    ret

But again labels are hard (for me ) in md.


> Of course that depends on zicond and zbs.  So we probably want the 
> expansion to not depend on those extensions, but generate code that is 
> easily recognized and converted into that kind of a sequence.

Is this a common enough paradigm: {bimodal,trimodal} values based on
{2,3} conditions. If so we could do a helper for baseline and then
optimization.
Otherwise I can just hack up isinf conditional to zicond and zbs based
on your code above - after all both of these extensions are likely to be
fairly common going fwd.

Thx for the quick feedback.

-Vineet

Reply via email to