Avoid inlining calls to external functions

External functions don't have bodies to inline anyway.
Fixes https://github.com/KhronosGroup/SPIRV-Tools/issues/605

diff --git a/source/opt/inline_pass.cpp b/source/opt/inline_pass.cpp
index 26dd4a3..8be3ef9 100644 (file)
--- a/source/opt/inline_pass.cpp
+++ b/source/opt/inline_pass.cpp
@@ -351,12 +351,20 @@ void InlinePass::GenInlineCode(
   }
 }
 
+bool InlinePass::IsInlinableFunctionCall(const ir::Instruction* inst) {
+  if (inst->opcode() != SpvOp::SpvOpFunctionCall) return false;
+  const ir::Function* calleeFn =
+      id2function_[inst->GetSingleWordOperand(kSpvFunctionCallFunctionId)];
+  // We can only inline a function if it has blocks.
+  return calleeFn->cbegin() != calleeFn->cend();
+}
+
 bool InlinePass::Inline(ir::Function* func) {
   bool modified = false;
   // Using block iterators here because of block erasures and insertions.
   for (auto bi = func->begin(); bi != func->end(); bi++) {
     for (auto ii = bi->begin(); ii != bi->end();) {
-      if (ii->opcode() == SpvOp::SpvOpFunctionCall) {
+      if (IsInlinableFunctionCall(&*ii)) {
         // Inline call.
         std::vector<std::unique_ptr<ir::BasicBlock>> newBlocks;
         std::vector<std::unique_ptr<ir::Instruction>> newVars;
@@ -368,8 +376,8 @@ bool InlinePass::Inline(ir::Function* func) {
           const auto lastBlk = newBlocks.end() - 1;
           const uint32_t firstId = (*firstBlk)->label_id();
           const uint32_t lastId = (*lastBlk)->label_id();
-          (*lastBlk)
-              ->ForEachSuccessorLabel([&firstId, &lastId, this](uint32_t succ) {
+          (*lastBlk)->ForEachSuccessorLabel(
+              [&firstId, &lastId, this](uint32_t succ) {
                 ir::BasicBlock* sbp = this->id2block_[succ];
                 sbp->ForEachPhiInst([&firstId, &lastId](ir::Instruction* phi) {
                   phi->ForEachInId([&firstId, &lastId](uint32_t* id) {

diff --git a/source/opt/inline_pass.h b/source/opt/inline_pass.h
index 523e193..541695b 100644 (file)
--- a/source/opt/inline_pass.h
+++ b/source/opt/inline_pass.h
@@ -107,7 +107,7 @@ class InlinePass : public Pass {
   // is returned. Formal parameters are trivially mapped to their actual
   // parameters. Note that the first block in new_blocks retains the label
   // of the original calling block. Also note that if an exit block is
-  // created, it is the last block of new_blocks. 
+  // created, it is the last block of new_blocks.
   //
   // Also return in new_vars additional OpVariable instructions required by
   // and to be inserted into the caller function after the block at
@@ -117,6 +117,9 @@ class InlinePass : public Pass {
                      ir::UptrVectorIterator<ir::Instruction> call_inst_itr,
                      ir::UptrVectorIterator<ir::BasicBlock> call_block_itr);
 
+  // Returns true if |inst| is a function call that can be inlined.
+  bool IsInlinableFunctionCall(const ir::Instruction* inst);
+
   // Exhaustively inline all function calls in func as well as in
   // all code that is inlined into func. Return true if func is modified.
   bool Inline(ir::Function* func);

diff --git a/test/opt/inline_test.cpp b/test/opt/inline_test.cpp
index 66eb176..ea578ac 100644 (file)
--- a/test/opt/inline_test.cpp
+++ b/test/opt/inline_test.cpp
@@ -1358,6 +1358,30 @@ TEST_F(InlineTest, OpImageAndOpSampledImageOutOfBlock) {
       /* skip_nop = */ false, /* do_validate = */ true);
 }
 
+TEST_F(InlineTest, ExternalFunctionIsNotInlined) {
+  // In particular, don't crash.
+  // See report https://github.com/KhronosGroup/SPIRV-Tools/issues/605
+  const std::string assembly =
+      R"(OpCapability Addresses
+OpCapability Kernel
+OpCapability Linkage
+OpMemoryModel Physical32 OpenCL
+OpEntryPoint Kernel %1 "entry_pt"
+OpDecorate %2 LinkageAttributes "external" Import
+%void = OpTypeVoid
+%4 = OpTypeFunction %void
+%2 = OpFunction %void None %4
+OpFunctionEnd
+%1 = OpFunction %void None %4
+%5 = OpLabel
+%6 = OpFunctionCall %void %2
+OpReturn
+OpFunctionEnd
+)";
+
+  SinglePassRunAndCheck<opt::InlinePass>(assembly, assembly, false, true);
+}
+
 // TODO(greg-lunarg): Add tests to verify handling of these cases:
 //
 //    Empty modules