This is an archive of the discontinued LLVM Phabricator instance.

Differential D65468

[NVPTX] Fix PR41651
ClosedPublic

Authored by hliao on Jul 30 2019, 12:12 PM.

Details

Reviewers
jholewinski
timshen
tra
Commits
rGf3983cc14af3: [NVPTX] Fix PR41651
rL367349: [NVPTX] Fix PR41651
Summary
  • Use the reliable way to retrive the IR module from callsite. As the indirect callsite doesn't have a called function, it would crash the compiler. Retrive the module from CS's call instruction instead.

Diff Detail

Event Timeline

hliao created this revision.Jul 30 2019, 12:12 PM
Herald added a project: Restricted Project. · View Herald TranscriptJul 30 2019, 12:12 PM
Herald added subscribers: llvm-commits, hiraditya, jholewinski. · View Herald Transcript
Harbormaster completed remote builds in B35826: Diff 212407.Jul 30 2019, 12:12 PM
hliao added reviewers: jholewinski, timshen.Jul 30 2019, 12:14 PM
tra added a reviewer: tra.Jul 30 2019, 12:21 PM
tra added inline comments.Jul 30 2019, 12:25 PM
llvm/lib/Target/NVPTX/NVPTXISelLowering.cpp
1296

Can we just use DAG.getDataLayout() as we do everywhere else?

hliao marked an inline comment as done.Jul 30 2019, 12:39 PM
hliao added inline comments.
llvm/lib/Target/NVPTX/NVPTXISelLowering.cpp
1296

Unfortunately, either DAG or CLI (CallLoweringInfo) is not passed as parameter. Without changing prototype of that function, get data layout from CS is the trivial way.

hliao marked an inline comment as done.Jul 30 2019, 12:41 PM
hliao added inline comments.
llvm/lib/Target/NVPTX/NVPTXISelLowering.cpp
1296

Wait, DL is passed as parameter, I will simplify the patch

hliao updated this revision to Diff 212413.Jul 30 2019, 12:43 PM

simplify the patch

tra accepted this revision.Jul 30 2019, 12:46 PM
tra added inline comments.
llvm/lib/Target/NVPTX/NVPTXISelLowering.cpp
1296

Even better! :-)

This revision is now accepted and ready to land.Jul 30 2019, 12:46 PM
Harbormaster completed remote builds in B35829: Diff 212413.Jul 30 2019, 12:47 PM
Closed by commit rL367349: [NVPTX] Fix PR41651 (authored by hliao). · Explain WhyJul 30 2019, 12:53 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents

PathSize
llvm/
lib/
Target/
NVPTX/
4 lines
test/
CodeGen/
NVPTX/
13 lines

Diff 212413

llvm/lib/Target/NVPTX/NVPTXISelLowering.cpp

Show First 20 Lines • Show All 1,285 Lines • ▼ Show 20 Lines if (retTy->isFloatingPointTy() || (retTy->isIntegerTy() && !retTy->isIntegerTy(128))) {
// bits in size. fp16 normally uses .b16 as its storage type in // bits in size. fp16 normally uses .b16 as its storage type in
// PTX, so its size must be adjusted here, too. // PTX, so its size must be adjusted here, too.
if (size < 32) if (size < 32)
size = 32; size = 32;
O << ".param .b" << size << " _"; O << ".param .b" << size << " _";
} else if (isa<PointerType>(retTy)) { } else if (isa<PointerType>(retTy)) {
O << ".param .b" << PtrVT.getSizeInBits() << " _"; O << ".param .b" << PtrVT.getSizeInBits() << " _";
} else if (retTy->isAggregateType() || retTy->isVectorTy() || retTy->isIntegerTy(128)) { } else if (retTy->isAggregateType() || retTy->isVectorTy() ||
auto &DL = CS.getCalledFunction()->getParent()->getDataLayout(); retTy->isIntegerTy(128)) {
O << ".param .align " << retAlignment << " .b8 _[" O << ".param .align " << retAlignment << " .b8 _["
traUnsubmitted
Not Done
ReplyInline Actions

Can we just use DAG.getDataLayout() as we do everywhere else?

tra: Can we just use `DAG.getDataLayout()` as we do everywhere else?
hliaoAuthorUnsubmitted
Done
ReplyInline Actions

Unfortunately, either DAG or CLI (CallLoweringInfo) is not passed as parameter. Without changing prototype of that function, get data layout from CS is the trivial way.

hliao: Unfortunately, either DAG or CLI (CallLoweringInfo) is not passed as parameter. Without…
hliaoAuthorUnsubmitted
Done
ReplyInline Actions

Wait, DL is passed as parameter, I will simplify the patch

hliao: Wait, DL is passed as parameter, I will simplify the patch
traUnsubmitted
Not Done
ReplyInline Actions

Even better! :-)

tra: Even better! :-)
<< DL.getTypeAllocSize(retTy) << "]"; << DL.getTypeAllocSize(retTy) << "]";
} else { } else {
llvm_unreachable("Unknown return type"); llvm_unreachable("Unknown return type");
} }
O << ") "; O << ") ";
} }
O << "_ ("; O << "_ (";
▲ Show 20 Lines • Show All 3,761 Lines • Show Last 20 Lines

llvm/test/CodeGen/NVPTX/bug41651.ll

  • This file was added.
; RUN: llc -filetype=asm -o - %s | FileCheck %s
target datalayout = "e-i64:64-i128:128-v16:16-v32:32-n16:32:64"
target triple = "nvptx64-nvidia-cuda"
%func = type { i32 (i32, i32)** }
; CHECK: foo
; CHECK: call
; CHECK: ret
define void @foo() {
%call = call %func undef(i32 0, i32 1)
ret void
}