On Wed, 2020-07-08 at 12:59 -0700, Carl Love wrote:
> [PATCH 5/6] rs6000, Add vector splat builtin support
> 
> ----------------------------------
> V4 Fixes:
> 
>    Rebased on mainline.  Changed FUTURE to P10.
>    define_predicate "s32bit_cint_operand" removed unnecessary cast in
>      definition.
>    Changed define_expand "xxsplti32dx_v4si" to use "0" for constraint
>      of operand 1.
>    Changed define_insn "xxsplti32dx_v4si_inst" to use "0 for constraint
>      of operand 1.
>    Removed define_predicate "f32bit_const_operand".  Use const_double_operand
>      instead.
> 
>    *** Please provide feedback for the following change:
>    (define_insn "xxspltidp_v2df_inst", Added print statement to warn of
>    possible undefined behavior.  The xxspltidp instruction result is
>    undefined for subnormal inputs.  I added a test for subnormal input with
>    a fprintf to stderr to warn the "user" if the constant input is a subnormal
>    value.  I tried assert initially, but that causes GCC to exit ungracefully
>    with no information as to why.  I really didn't like that behavior.
>    A subnormal input is not really a fatal error but the "user" needs
>    to be told it is not a good idea.  Not sure if using an fprintf statement
>    in a define_insn is an acceptable thing either.  But it does give the
>    user the needed input and GCC exits normally.  Let me know if there
>    is a better option here.


Maybe this should be an RFC/Patch then..   Put this in the intro
paragraph, not in the v4 fixes blurb. 


I certainly defer to other opinions here.
I don't see any other define_insn entries that emit warning printfs for 
undefined results.
I'd lean towards dropping that part here.

You may be able to add some logic over in rs6000-call.c  rs6000_expand_* to 
handle this.
Though I think there are many cases where undefined results can be the result 
of user input.
Perhaps just documenting this (extend.texi) is enough? 


<snip>
Nothing else below jumped out at me.

thanks, 
-Will



Reply via email to