"Robin Dapp" <[email protected]> writes:
>> Could we instead do something like:
>>
>> diff --git a/gcc/expmed.cc b/gcc/expmed.cc
>> index 13803c6bbd0..c6494484251 100644
>> --- a/gcc/expmed.cc
>> +++ b/gcc/expmed.cc
>> @@ -2022,14 +2022,16 @@ extract_integral_bit_field (rtx op0,
>> opt_scalar_int_mode op0_mode,
>> (unsignedp ? 1 : -1), target_part,
>> mode, word_mode, reverse, fallback_p, NULL);
>>
>> - gcc_assert (target_part);
>> if (!result_part)
>> {
>> delete_insns_since (last);
>> return NULL;
>> }
>>
>> - if (result_part != target_part)
>> + if (!target_part)
>> + store_bit_field (target, BITS_PER_WORD, i * BITS_PER_WORD,
>> + 0, 0, word_mode, result_part, false, i == 0);
>> + else if (result_part != target_part)
>> emit_move_insn (target_part, result_part);
>> }
>>
>> It fixes the ICE but I haven't tested beyond that.
>
> This is almost exactly what I had initially until I realized the
> operand_subword in the if (unsignedp). Even though I didn't manage to
> trigger it, it looked vulnerable.
If we hit the same problem there, I think the fix would be similar.
> But if you're OK with the above, I'll go ahead with that. It's even
> bootstrapped and regtested already :)
Yeah, I think the above is better. It's simpler, avoids a stack temporary,
and at least stands a chance of using target optabs to insert the
individual pieces.
>>> diff --git a/gcc/testsuite/g++.target/riscv/rvv/autovec/pr127323.C
>>> b/gcc/testsuite/g++.target/riscv/rvv/autovec/pr127323.C
>>> new file mode 100644
>>> index 00000000000..3809d81b6fe
>>> --- /dev/null
>>> +++ b/gcc/testsuite/g++.target/riscv/rvv/autovec/pr127323.C
>>> @@ -0,0 +1,39 @@
>>> +/* { dg-do compile } */
>>> +/* { dg-options "-O3 -mrvv-max-lmul=conv-dynamic -march=rv64gcv
>>> -mabi=lp64d -mvector-strict-align" } */
>>
>> For me the ICE only triggered with -mtune=generic-ooo, as in the PR.
>
> Yes, and that's due to a riscv target issue. Our
> riscv_slow_unaligned_access is currently wrong and sends generic-ooo the
> "unsupported" way even though it should do the opposite.
> I think, strictly, this particular ICE wouldn't even happen anymore
> after fixing riscv_slow_unaligned_access, regardless of the option.
> We aren't going to see element-misaligned vectors here and only those
> are unsupported.
>
> So yeah, I guess the test is then misleading at best. Maybe I should
> even drop the patch then, as we won't hit this particular case right
> now? It's still wrong as written I guess.
I think we should keep the patch and if possible apply it while
generic-ooo still exposes the issue, with a testcase that uses
generic-ooo.
If the generic-ooo options would no longer be a good choice once the
target issue is fixed, maybe it'd be worth keeping the above as a second
testcase that #includes the first? Just the generic-ooo test would be
ok with me though.
Thanks,
Richard
