Skip to content
This repository was archived by the owner on Sep 2, 2018. It is now read-only.

Support MULH[US] for all integer types #190

Merged
merged 2 commits into from
May 17, 2016
Merged

Support MULH[US] for all integer types #190

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
16327d4
Make sure we are looking at an instruction with register operands
shepmaster May 8, 2016
211dd8f
Expand MULHS for all types
shepmaster May 8, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 17 additions & 5 deletions lib/Target/AVR/AVRISelLowering.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,8 +137,11 @@ AVRTargetLowering::AVRTargetLowering(AVRTargetMachine &tm)
// Expand 16 bit multiplications.
setOperationAction(ISD::SMUL_LOHI, MVT::i16, Expand);
setOperationAction(ISD::UMUL_LOHI, MVT::i16, Expand);
setOperationAction(ISD::MULHS, MVT::i16, Expand);
setOperationAction(ISD::MULHU, MVT::i16, Expand);

for (MVT VT : MVT::integer_valuetypes()) {
setOperationAction(ISD::MULHS, VT, Expand);
setOperationAction(ISD::MULHU, VT, Expand);
}

// Runtime library functions
{
Expand Down Expand Up @@ -232,6 +235,12 @@ const char *AVRTargetLowering::getTargetNodeName(unsigned Opcode) const {
}
}

EVT AVRTargetLowering::getSetCCResultType(const DataLayout &DL, LLVMContext &,
EVT VT) const {
assert(!VT.isVector() && "No AVR SetCC type for vectors!");
return MVT::i8;
}

SDValue AVRTargetLowering::LowerShifts(SDValue Op, SelectionDAG &DAG) const {
//:TODO: this function has to be completely rewritten to produce optimal
// code, for now it's producing very long but correct code.
Expand Down Expand Up @@ -1474,9 +1483,12 @@ MachineBasicBlock *AVRTargetLowering::insertShift(MachineInstr *MI,
}

inline bool isCopyMulResult(MachineBasicBlock::iterator const &I) {
unsigned SrcReg = I->getOperand(1).getReg();
return I->getOpcode() == AVR::COPY &&
(SrcReg == AVR::R0 || SrcReg == AVR::R1);
if (I->getOpcode() == AVR::COPY) {
unsigned SrcReg = I->getOperand(1).getReg();
return (SrcReg == AVR::R0 || SrcReg == AVR::R1);
}

return false;
}

// The mul instructions wreak havock on our zero_reg R1. We need to clear it
Expand Down
3 changes: 3 additions & 0 deletions lib/Target/AVR/AVRISelLowering.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,9 @@ class AVRTargetLowering : public TargetLowering {

bool isOffsetFoldingLegal(const GlobalAddressSDNode *GA) const override;

EVT getSetCCResultType(const DataLayout &DL, LLVMContext &Context,
EVT VT) const override;

MachineBasicBlock *
EmitInstrWithCustomInserter(MachineInstr *MI,
MachineBasicBlock *MBB) const override;
Expand Down
33 changes: 0 additions & 33 deletions test/CodeGen/AVR/issue-cannot-select-mulhs.ll
View file
Open in desktop

This file was deleted.

31 changes: 31 additions & 0 deletions test/CodeGen/AVR/smul-with-overflow.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
; RUN: llc < %s -march=avr | FileCheck %s

define i1 @signed_multiplication_did_overflow(i8, i8) unnamed_addr {
; CHECK-LABEL: signed_multiplication_did_overflow:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would you be able to add CHECK and CHECK-NEXT lines to make sure the correct sequence of instructions is generated? Just something basic would suffice.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

signed_multiplication_did_overflow:     ; @signed_multiplication_did_overflow
; BB#0:                                 ; %entry-block
    muls    r24, r22
    mov r25, r1
    eor r1, r1
    mov r18, r0
    asr r18
    asr r18
    asr r18
    asr r18
    asr r18
    asr r18
    asr r18
    ldi r24, 1
    cp  r25, r18
    brne    LBB0_2
; BB#1:                                 ; %entry-block
    ldi r24, 0
LBB0_2:                                 ; %entry-block
    ret

This is a bit more involved than unsigned. What shape of tests would you like to see here?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something like this would be good (with patterns instead of registers):

define i1 @ signed_multiplication_did_overflow(i8, i8) unnamed_addr {
; CHECK-LABEL: signed_multiplication_did_overflow:
entry-block:
  %2 = tail call { i8, i1 } @llvm.umul.with.overflow.i8(i8 %0, i8 %1)
  %3 = extractvalue { i8, i1 } %2, 1
  ret i1 %3
; CHECK-NEXT: muls r24, r22
; CHECK-NEXT: mov r25, r1
; ...
}

declare { i8, i1 } @llvm.umul.with.overflow.i8(i8, i8)

Of course, don't worry about any calling convention or prologue/epilogue junk.

Same with the other test btw

entry-block:
%2 = tail call { i8, i1 } @llvm.smul.with.overflow.i8(i8 %0, i8 %1)
%3 = extractvalue { i8, i1 } %2, 1
ret i1 %3

; Multiply, fill the low byte with the sign of the low byte via
; arithmetic shifting, compare it to the high byte.
;
; CHECK: muls r24, r22
; CHECK: mov [[HIGH:r[0-9]+]], r1
; CHECK: mov [[LOW:r[0-9]+]], r0
; CHECK: asr {{.*}}[[LOW]]
; CHECK: asr {{.*}}[[LOW]]
; CHECK: asr {{.*}}[[LOW]]
; CHECK: asr {{.*}}[[LOW]]
; CHECK: asr {{.*}}[[LOW]]
; CHECK: asr {{.*}}[[LOW]]
; CHECK: asr {{.*}}[[LOW]]
; CHECK: ldi [[RET:r[0-9]+]], 1
; CHECK: cp {{.*}}[[HIGH]], {{.*}}[[LOW]]
; CHECK: brne [[LABEL:LBB[_0-9]+]]
; CHECK: ldi {{.*}}[[RET]], 0
; CHECK: {{.*}}[[LABEL]]
; CHECK: ret
}

declare { i8, i1 } @llvm.smul.with.overflow.i8(i8, i8)
22 changes: 22 additions & 0 deletions test/CodeGen/AVR/umul-with-overflow.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
; RUN: llc < %s -march=avr | FileCheck %s

define i1 @unsigned_multiplication_did_overflow(i8, i8) unnamed_addr {
; CHECK-LABEL: unsigned_multiplication_did_overflow:
entry-block:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The assembly:

unsigned_multiplication_did_overflow:   ; @unsigned_multiplication_did_overflow
; BB#0:                                 ; %entry-block
    mul r24, r22
    mov r25, r1
    eor r1, r1
    ldi r24, 1
    cpi r25, 0
    brne    LBB0_2
; BB#1:                                 ; %entry-block
    ldi r24, 0
LBB0_2:                                 ; %entry-block
    ret

This one is straight-forward - do the multiply and then check to see if the high byte is zero. There's some small inefficiencies. What general shape of tests would you like to see?

%2 = tail call { i8, i1 } @llvm.umul.with.overflow.i8(i8 %0, i8 %1)
%3 = extractvalue { i8, i1 } %2, 1
ret i1 %3

; Multiply, return if the high byte is zero
;
; CHECK: mul r{{[0-9]+}}, r{{[0-9]+}}
; CHECK: mov [[HIGH:r[0-9]+]], r1
; CHECK: ldi [[RET:r[0-9]+]], 1
; CHECK: cpi {{.*}}[[HIGH]], 0
; CHECK: brne [[LABEL:LBB[_0-9]+]]
; CHECK: ldi {{.*}}[[RET]], 0
; CHECK: {{.*}}[[LABEL]]
; CHECK: ret
}

declare { i8, i1 } @llvm.umul.with.overflow.i8(i8, i8)