[PATCH] D128572: [LoongArch] Add codegen support for division operations

[Xi Ruoyao via Phabricator via llvm-commits]

xry111 added a comment.

It seems I can't remove an inline comment...



================
Comment at: llvm/test/CodeGen/LoongArch/ir-instruction/sdiv-udiv-srem-urem.ll:132
+; LA64-NOTRAP-NEXT:    addi.w $a0, $a0, 0
+; LA64-NOTRAP-NEXT:    div.d $a0, $a0, $a1
+; LA64-NOTRAP-NEXT:    jirl $zero, $ra, 0
----------------
xry111 wrote:
> xen0n wrote:
> > SixWeining wrote:
> > > xry111 wrote:
> > > > It looks suboptimal: "div.w $a0, $a0, $a1" should work so these two sign-extensions are not needed.
> > > > 
> > > > I'm not sure if it's easy to optimize this.  If an optimization is not suitable for this revision, we can do it later.
> > > Yes. 32bit division can be optimized to div.w but we must make sure the inputs are sign extend values. This limitation is marked in Chinese ISA document but not in the English document. Maybe the English version is outdated.
> > > {F23575515}
> > > 
> > > ```
> > > define i32 @sdiv_i32(i32 %a, i32 %b) {
> > > entry:
> > >   %r = sdiv i32 %a, %b
> > >   ret i32 %r
> > > }
> > > =>
> > > ; LA64-NOTRAP-NEXT:    addi.w $a1, $a1, 0
> > > ; LA64-NOTRAP-NEXT:    addi.w $a0, $a0, 0
> > > ; LA64-NOTRAP-NEXT:    div.w $a0, $a0, $a1
> > > ```
> > > 
> > > ```
> > > define i32 @sdiv_i32(i32 signext %a, i32 signext %b) {
> > > entry:
> > >   %r = sdiv i32 %a, %b
> > >   ret i32 %r
> > > }
> > > =>
> > > ; LA64-NOTRAP-NEXT:    div.w $a0, $a0, $a1
> > > ```
> > > 
> > > Since this is an improvement to the codegen, let me implement it with seperate patch in future. Thanks.
> > Indeed; however the limitation seems hugely likely a hardware erratum, because almost any other 32-bit operation on LA64 silently ignores the upper bits. I think it's better to confirm with the hardware team, in case this is another error fixed in the translation.
> GCC simply generates a div.w instruction.
My mistake: I saw GCC generated "div.w $a0, $a0, $a1", but it only works because in the ABI parameters are already signed-extended.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128572