This is an archive of the discontinued LLVM Phabricator instance.

Differential D105536

[M68k][GloballSel] LegalizerInfo implementation
ClosedPublic

Authored by sushmaunnibhavi on Jul 7 2021, 1:18 AM.

Details

Reviewers
myhsu
gandhi21299
Commits
rGaaccc985a88d: [M68k][GloballSel] LegalizerInfo implementation
Summary

Added rules for G_ADD, G_SUB, G_MUL, G_UDIV to be legal.

Diff Detail

Unit TestsFailed

TimeTest
2,730 msx64 debian > libarcher.barrier::barrier.c
2,730 msx64 debian > libarcher.critical::critical.c
2,930 msx64 debian > libarcher.critical::lock.c
2,970 msx64 debian > libarcher.parallel::parallel-firstprivate.c
2,920 msx64 debian > libarcher.races::critical-unrelated.c
View Full Test Results (19 Failed)

Event Timeline

sushmaunnibhavi created this revision.Jul 7 2021, 1:18 AM
Herald added a subscriber: hiraditya. · View Herald TranscriptJul 7 2021, 1:18 AM
sushmaunnibhavi requested review of this revision.Jul 7 2021, 1:18 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 7 2021, 1:18 AM
Herald added a subscriber: llvm-commits. · View Herald Transcript
Harbormaster completed remote builds in B112745: Diff 356891.Jul 7 2021, 2:53 AM
gandhi21299 added a comment.Jul 7 2021, 9:55 AM

More descriptive filename and please address the lint comments. Other than that, I will count on @myhsu regarding correctness of gMIR.

sushmaunnibhavi added a comment.Jul 10 2021, 7:06 AM
In D105536#2862185, @gandhi21299 wrote:

More descriptive filename and please address the lint comments. Other than that, I will count on @myhsu regarding correctness of gMIR.

Ok I will do it.
@myhsu can you please review the patch?

myhsu added a comment.Jul 11 2021, 11:33 AM

LGTM on the technical side, please address @gandhi21299 's comments.
Though this patch is definitely not production ready, it can be a good reference for future implementation on GlobalISel in M68k :-)

sushmaunnibhavi updated this revision to Diff 357860.Jul 12 2021, 1:37 AM

Updated patch according to @gandhi21299, @myhsu comments.

xgupta added a subscriber: xgupta.Jul 12 2021, 2:03 AM

(you have permission to disagree with clang-tidy, it lacks A.I.)

Harbormaster completed remote builds in B113452: Diff 357860.Jul 12 2021, 2:20 AM
sushmaunnibhavi updated this revision to Diff 357878.Jul 12 2021, 3:12 AM
In D105536#2870581, @xgupta wrote:

(you have permission to disagree with clang-tidy, it lacks A.I.)

Okay!

Harbormaster completed remote builds in B113464: Diff 357878.Jul 12 2021, 3:51 AM
sushmaunnibhavi updated this revision to Diff 358139.Jul 12 2021, 9:15 PM

Fix clang-tidy warnings.

xgupta removed a subscriber: xgupta.Jul 12 2021, 9:45 PM
Harbormaster completed remote builds in B113646: Diff 358139.Jul 12 2021, 9:49 PM
glaubitz added a subscriber: glaubitz.Jul 13 2021, 4:11 AM
myhsu accepted this revision.Jul 14 2021, 2:54 PM

LGTM

This revision is now accepted and ready to land.Jul 14 2021, 2:54 PM
sushmaunnibhavi added a comment.Jul 14 2021, 7:34 PM
In D105536#2878456, @myhsu wrote:

LGTM

Thanks for reviewing the patch @myhsu.
Can you please commit the patch?

This revision was landed with ongoing or failed builds.Jul 15 2021, 12:01 PM
Closed by commit rGaaccc985a88d: [M68k][GloballSel] LegalizerInfo implementation (authored by sushmaunnibhavi, committed by gandhi21299). · Explain Why
This revision was automatically updated to reflect the committed changes.
gandhi21299 added a commit: rGaaccc985a88d: [M68k][GloballSel] LegalizerInfo implementation.

Revision Contents

PathSize
llvm/
lib/
Target/
M68k/
GlSel/
9 lines
test/
CodeGen/
M68k/
GlobalISel/
57 lines

Diff 357878

llvm/lib/Target/M68k/GlSel/M68kLegalizerInfo.cpp

Show All 14 Lines
#include "llvm/CodeGen/TargetOpcodes.h" #include "llvm/CodeGen/TargetOpcodes.h"
#include "llvm/CodeGen/ValueTypes.h" #include "llvm/CodeGen/ValueTypes.h"
#include "llvm/IR/DerivedTypes.h" #include "llvm/IR/DerivedTypes.h"
#include "llvm/IR/Type.h" #include "llvm/IR/Type.h"
using namespace llvm; using namespace llvm;
M68kLegalizerInfo::M68kLegalizerInfo(const M68kSubtarget &ST) { M68kLegalizerInfo::M68kLegalizerInfo(const M68kSubtarget &ST) {
using namespace TargetOpcode;
const LLT s32 = LLT::scalar(32);
Lint: Pre-merge checks
Inline Actions

clang-tidy: warning: invalid case style for variable 's32' [readability-identifier-naming]
not useful

Lint: Pre-merge checks: clang-tidy: warning: invalid case style for variable 's32' [readability-identifier-naming]…
const LLT p0 = LLT::pointer(0, 32);
Lint: Pre-merge checks
Inline Actions

clang-tidy: warning: invalid case style for variable 'p0' [readability-identifier-naming]
not useful

Lint: Pre-merge checks: clang-tidy: warning: invalid case style for variable 'p0' [readability-identifier-naming]…
getActionDefinitionsBuilder(G_LOAD).legalFor({s32});
getActionDefinitionsBuilder(G_FRAME_INDEX).legalFor({p0});
getActionDefinitionsBuilder(G_ADD).legalFor({s32});
getActionDefinitionsBuilder(G_SUB).legalFor({s32});
getActionDefinitionsBuilder(G_MUL).legalFor({s32});
getActionDefinitionsBuilder(G_UDIV).legalFor({s32});
getLegacyLegalizerInfo().computeTables(); getLegacyLegalizerInfo().computeTables();
} }

llvm/test/CodeGen/M68k/GlobalISel/arithmetic.ll

  • This file was added.
; RUN: llc -mtriple=m68k -global-isel -stop-after=legalizer < %s | FileCheck %s
define i32 @test_add(i32 %x, i32 %y) {
; CHECK-LABEL: name: test_add
; CHECK: bb.1 (%ir-block.0):
; CHECK: [[G_F_I1:%[0-9]+]]:_(p0) = G_FRAME_INDEX
; CHECK: [[G_LOAD1:%[0-9]+]]:_(s32) = G_LOAD [[G_F_I1]](p0)
; CHECK: [[G_F_I2:%[0-9]+]]:_(p0) = G_FRAME_INDEX
; CHECK: [[G_LOAD2:%[0-9]+]]:_(s32) = G_LOAD [[G_F_I2]](p0)
; CHECK: [[G_ADD1:%[0-9]+]]:_(s32) = G_ADD [[G_LOAD1]], [[G_LOAD2]]
; CHECK: $d0 = COPY [[G_ADD1]](s32)
; CHECK: RTS implicit $d0
%sum = add i32 %x, %y
ret i32 %sum
}
define i32 @test_sub(i32 %x, i32 %y) {
; CHECK-LABEL: name: test_sub
; CHECK: bb.1 (%ir-block.0):
; CHECK: [[G_F_I1:%[0-9]+]]:_(p0) = G_FRAME_INDEX
; CHECK: [[G_LOAD1:%[0-9]+]]:_(s32) = G_LOAD [[G_F_I1]](p0)
; CHECK: [[G_F_I2:%[0-9]+]]:_(p0) = G_FRAME_INDEX
; CHECK: [[G_LOAD2:%[0-9]+]]:_(s32) = G_LOAD [[G_F_I2]](p0)
; CHECK: [[G_SUB1:%[0-9]+]]:_(s32) = G_SUB [[G_LOAD1]], [[G_LOAD2]]
; CHECK: $d0 = COPY [[G_SUB1]](s32)
; CHECK: RTS implicit $d0
%diff = sub i32 %x, %y
ret i32 %diff
}
define i32 @test_mul(i32 %x, i32 %y) {
; CHECK-LABEL: name: test_mul
; CHECK: bb.1 (%ir-block.0):
; CHECK: [[G_F_I1:%[0-9]+]]:_(p0) = G_FRAME_INDEX
; CHECK: [[G_LOAD1:%[0-9]+]]:_(s32) = G_LOAD [[G_F_I1]](p0)
; CHECK: [[G_F_I2:%[0-9]+]]:_(p0) = G_FRAME_INDEX
; CHECK: [[G_LOAD2:%[0-9]+]]:_(s32) = G_LOAD [[G_F_I2]](p0)
; CHECK: [[G_MUL1:%[0-9]+]]:_(s32) = G_MUL [[G_LOAD1]], [[G_LOAD2]]
; CHECK: $d0 = COPY [[G_MUL1]](s32)
; CHECK: RTS implicit $d0
%prod = mul i32 %x, %y
ret i32 %prod
}
define i32 @test_udiv(i32 %x, i32 %y) {
; CHECK-LABEL: name: test_udiv
; CHECK: bb.1 (%ir-block.0):
; CHECK: [[G_F_I1:%[0-9]+]]:_(p0) = G_FRAME_INDEX
; CHECK: [[G_LOAD1:%[0-9]+]]:_(s32) = G_LOAD [[G_F_I1]](p0)
; CHECK: [[G_F_I2:%[0-9]+]]:_(p0) = G_FRAME_INDEX
; CHECK: [[G_LOAD2:%[0-9]+]]:_(s32) = G_LOAD [[G_F_I2]](p0)
; CHECK: [[G_DIV1:%[0-9]+]]:_(s32) = G_UDIV [[G_LOAD1]], [[G_LOAD2]]
; CHECK: $d0 = COPY [[G_DIV1]](s32)
; CHECK: RTS implicit $d0
%div = udiv i32 %x, %y
ret i32 %div
}