[PATCH] D149743: [RISCV][CodeGen] Support Zdinx on RV32 codegen

[Craig Topper via Phabricator via llvm-commits]

craig.topper added a comment.

In D149743#4316139 <https://reviews.llvm.org/D149743#4316139>, @jrtc27 wrote:

> In D149743#4315278 <https://reviews.llvm.org/D149743#4315278>, @sunshaoce wrote:
>
>> In D122918 <https://reviews.llvm.org/D122918>, @jrct27 said
>>
>>> I don't know why you've marked this as done; the line in question (which is now the line below) is not correct, the immediate could be 0x7f[cdef] at which point the arithmetic here will wrap to 0x80[0123] (which the machine verifier probably catches as out of range due to the immediate being signed?..). That is:
>>>
>>>   void
>>>   foo(void *p, double d)
>>>   {
>>>       *(double *)((char *)p + 0x7fc) = d;
>>>   }
>>>
>>> will be miscompiled, surely.
>>
>> I tested it now. It seems that the bug has already been fixed.
>>
>>   $ bin/clang -cc1 -triple riscv32 -target-feature +zdinx -S test.c -O3 
>>   
>>   foo:                                    # @foo
>>   .Lfoo$local:
>>   	.type	.Lfoo$local, at function
>>   # %bb.0:                                # %entry
>>   	addi	a3, a0, 2044
>>   	sw	a1, 2044(a0)
>>   	sw	a2, 4(a3)
>>   	ret
>>
>>
>>
>>   $ bin/clang -cc1 -triple riscv32 -S test.c -O3
>>   
>>   foo:                                    # @foo
>>   .Lfoo$local:
>>   	.type	.Lfoo$local, at function
>>   # %bb.0:                                # %entry
>>   	addi	a3, a0, 2044
>>   	sw	a1, 2044(a0)
>>   	sw	a2, 4(a3)
>>   	ret
>>
>>
>>
>>   $ bin/clang -cc1 -triple riscv32 -target-feature +d -S test.c -O3
>>   
>>   foo:                                    # @foo
>>   .Lfoo$local:
>>   	.type	.Lfoo$local, at function
>>   # %bb.0:                                # %entry
>>   	fsd	fa0, 2044(a0)
>>   	ret
>>
>> Any suggestions？@jrtc27
>
> At the IR level (and DAG level) is it actually doing an f64 store? I can’t see how your code would ever work for that case. I also don’t know if unaligned accesses of globalisation needs to work, because if so your code is similarly broken there.

I think returning an f64 load will get bitcasted by the ABI which will convert the load to an i64 load which will go to type legalization. You need to test a load followed by an FP operation like fadd.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D149743/new/

https://reviews.llvm.org/D149743