Ian Romanick <i...@freedesktop.org> writes: > On 01/24/2017 03:26 PM, Francisco Jerez wrote: >> This does point at the front-end emitting silly code that could have >> been optimized out, but the current fsign implementation would emit >> bogus IR if abs was set for the argument (because it would apply the >> abs modifier on an unsigned integer type), and we shouldn't rely on >> the upper layer's optimization passes for correctness. > > Other than the atan2 code you emit later in the series, is there a test > for this? >
PATCH 5 would cause a pile of atan tests to regress without this, but
see the attachment for a test-case that reproduces the problem in
isolation.
>> ---
>> src/mesa/drivers/dri/i965/brw_fs_nir.cpp | 9 ++++++++-
>> 1 file changed, 8 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
>> b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
>> index e1ab598..e0c2fa0 100644
>> --- a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
>> +++ b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
>> @@ -701,7 +701,14 @@ fs_visitor::nir_emit_alu(const fs_builder &bld,
>> nir_alu_instr *instr)
>> break;
>>
>> case nir_op_fsign: {
>> - if (type_sz(op[0].type) < 8) {
>> + if (op[0].abs) {
>> + /* Straightforward since the source can be assumed to be
>> + * non-negative.
>> + */
>> + set_condmod(BRW_CONDITIONAL_NZ, bld.MOV(result, op[0]));
>> + set_predicate(BRW_PREDICATE_NORMAL, bld.MOV(result,
>> brw_imm_f(1.0f)));
>
> Does this work for DF source?
>
Yup.
> If we had an optimization pass for this, it would probably map
> fsign(abs(a)) to float(a != 0) or double(a != 0). This is different
> from what we would generate for that, but I don't know which is better.
>
The main reason I did it that way was because it's able to handle double
or single precision as-is without any special cases -- To do double(a !=
0) you need a 2-src CMP instruction which means you cannot use a DF
immediate to compare against. float(a != 0) is probably marginally
better for single-precision though, because the second instruction would
have a higher chance of getting optimized out -- If you like I'll make
the change and deal with the restrictions of DF immediates.
>> +
>> + } else if (type_sz(op[0].type) < 8) {
>> /* AND(val, 0x80000000) gives the sign bit.
>> *
>> * Predicated OR ORs 1.0 (0x3f800000) with the sign bit if val is
>> not
>>
sign-abs.shader_test
Description: Binary data
signature.asc
Description: PGP signature
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
