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