This is an archive of the discontinued LLVM Phabricator instance.

Differential D110555

[SPARC] Zero-extend the operands when doing UMULO on 64-bit integers
ClosedPublic

Authored by koakuma on Sep 27 2021, 8:11 AM.

Details

Reviewers
venkatra
dcederman
LemonBoy
Commits
rG3e0f3041cc1a: [SPARC] Zero-extend the operands when doing UMULO on 64-bit integers
Summary

On SPARC, S/UMULO operation on 64-bit integers works by extending the value to 128-bit, then doing a multiplication and checking the upper half of the result.
This makes UMULO works correctly by putting a zero in the upper half rather than doing a sign extension.

Diff Detail

Event Timeline

koakuma created this revision.Sep 27 2021, 8:11 AM
Herald added subscribers: jrtc27, fedor.sergeev, hiraditya, jyknight. · View Herald TranscriptSep 27 2021, 8:11 AM
koakuma requested review of this revision.Sep 27 2021, 8:11 AM
Herald added a project: Restricted Project. · View Herald TranscriptSep 27 2021, 8:11 AM
Herald added a subscriber: llvm-commits. · View Herald Transcript
Harbormaster completed remote builds in B125876: Diff 375263.Sep 27 2021, 8:28 AM
koakuma updated this revision to Diff 375286.Sep 27 2021, 8:37 AM

Fix clang-format errors.

Harbormaster completed remote builds in B125891: Diff 375286.Sep 27 2021, 9:52 AM
koakuma updated this revision to Diff 377813.Oct 7 2021, 5:36 AM

Also fix the other testcases affected by this change, since they happen to contain the wrong behavior too.

Harbormaster completed remote builds in B127495: Diff 377813.Oct 7 2021, 5:37 AM
brad added reviewers: dcederman, LemonBoy.Oct 17 2021, 8:55 PM
LemonBoy added a comment.Oct 28 2021, 2:36 AM

Please upload the diff with full context, check out the contribution guide for more info.

koakuma updated this revision to Diff 383326.Oct 29 2021, 5:55 AM

Updated patch with full context.

Harbormaster completed remote builds in B131407: Diff 383326.Oct 29 2021, 6:34 AM
koakuma updated this revision to Diff 383503.Oct 29 2021, 2:37 PM

clang-format fixes

Harbormaster completed remote builds in B131519: Diff 383503.Oct 29 2021, 3:11 PM
LemonBoy added a comment.Nov 8 2021, 9:37 AM

The fix LGTM, there's only a minor nit wrt the test placement.

llvm/test/CodeGen/SPARC/mulo-64-lowering.ll
2

Generate this file using update_llc_test_checks.py, that way it's much easier to update it if needed.

Minor nit to keep the test suite tidy, the unsigned case is already handled in umulo-128-legalisation-lowering.ll, perhaps copy-paste that and call it smulo-128-legalisation-lowering.ll.

koakuma updated this revision to Diff 386490.Nov 11 2021, 5:53 AM

Changed the test files as per @LemonBoy's suggestion.

Harbormaster completed remote builds in B133706: Diff 386490.Nov 11 2021, 6:32 AM
koakuma updated this revision to Diff 386519.Nov 11 2021, 7:58 AM

Fix testcase generation.

Harbormaster completed remote builds in B133725: Diff 386519.Nov 11 2021, 8:31 AM
LemonBoy accepted this revision.Nov 13 2021, 3:01 AM

LGTM. I can land this for you if you have no commit access, just give me the name/email you want to be credited as.

This revision is now accepted and ready to land.Nov 13 2021, 3:01 AM
koakuma added a comment.Nov 14 2021, 6:47 AM
In D110555#3129145, @LemonBoy wrote:

LGTM. I can land this for you if you have no commit access, just give me the name/email you want to be credited as.

I have no commit access so please commit it for me. My email is koachan@protonmail.com.
Thanks a lot for the help.

Closed by commit rG3e0f3041cc1a: [SPARC] Zero-extend the operands when doing UMULO on 64-bit integers (authored by koakuma, committed by LemonBoy). · Explain WhyNov 14 2021, 11:00 AM
This revision was automatically updated to reflect the committed changes.
LemonBoy added a commit: rG3e0f3041cc1a: [SPARC] Zero-extend the operands when doing UMULO on 64-bit integers.

Revision Contents

PathSize
llvm/
lib/
Target/
Sparc/
11 lines
test/
CodeGen/
SPARC/
36 lines

Diff 375263

llvm/lib/Target/Sparc/SparcISelLowering.cpp

Context not available.
SDValue ShiftAmt = DAG.getConstant(63, dl, VT); SDValue ShiftAmt = DAG.getConstant(63, dl, VT);
SDValue RHS = Op.getOperand(1); SDValue RHS = Op.getOperand(1);
SDValue HiLHS = DAG.getNode(ISD::SRA, dl, VT, LHS, ShiftAmt); SDValue HiLHS, HiRHS;
SDValue HiRHS = DAG.getNode(ISD::SRA, dl, MVT::i64, RHS, ShiftAmt); if(isSigned) {
HiLHS = DAG.getNode(ISD::SRA, dl, VT, LHS, ShiftAmt);
HiRHS = DAG.getNode(ISD::SRA, dl, MVT::i64, RHS, ShiftAmt);
} else {
Lint: Pre-merge checks
Inline Actions

clang-format: please reformat the code

-  if(isSigned) {
+  if (isSigned) {
Lint: Pre-merge checks: clang-format: please reformat the code ``` - if(isSigned) { + if (isSigned) { ```
HiLHS = DAG.getConstant(0, dl, VT);
HiRHS = DAG.getConstant(0, dl, MVT::i64);
}
SDValue Args[] = { HiLHS, LHS, HiRHS, RHS }; SDValue Args[] = { HiLHS, LHS, HiRHS, RHS };
TargetLowering::MakeLibCallOptions CallOptions; TargetLowering::MakeLibCallOptions CallOptions;
Context not available.

llvm/test/CodeGen/SPARC/mulo-64-lowering.ll

  • This file was added.
; RUN: llc -march=sparcv9 < %s | FileCheck %s
LemonBoyUnsubmitted
Not Done
ReplyInline Actions

Generate this file using update_llc_test_checks.py, that way it's much easier to update it if needed.

Minor nit to keep the test suite tidy, the unsigned case is already handled in umulo-128-legalisation-lowering.ll, perhaps copy-paste that and call it smulo-128-legalisation-lowering.ll.

LemonBoy: Generate this file using `update_llc_test_checks.py`, that way it's much easier to update it if…
declare { i64, i1 } @llvm.smul.with.overflow.i64(i64, i64)
declare { i64, i1 } @llvm.umul.with.overflow.i64(i64, i64)
; CHECK-LABEL: s:
; CHECK: save %sp, -176, %sp
; CHECK: srax %i0, 63, %o0
; CHECK: srax %i1, 63, %o2
; CHECK: mov %i0, %o1
; CHECK: call __multi3
; CHECK: mov %i1, %o3
; CHECK: ret
; CHECK-NEXT: restore %g0, %o1, %o0
define i64 @s(i64 %0, i64 %1) {
%3 = call { i64, i1 } @llvm.smul.with.overflow.i64(i64 %0, i64 %1)
%4 = extractvalue { i64, i1 } %3, 0
ret i64 %4
}
; CHECK-label: u:
; CHECK: save %sp, -176, %sp
; CHECK: mov 0, %o0
; CHECK: mov %i0, %o1
; CHECK: mov %o0, %o2
; CHECK: call __multi3
; CHECK: mov %i1, %o3
; CHECK: ret
; CHECK-NEXT: restore %g0, %o1, %o0
define i64 @u(i64 %0, i64 %1) local_unnamed_addr #0 {
%3 = call { i64, i1 } @llvm.umul.with.overflow.i64(i64 %0, i64 %1)
%4 = extractvalue { i64, i1 } %3, 0
ret i64 %4
}