[WebAssembly] Add TargetInstrInfo::getCalleeOperand

[aheejin]

Summary

DwarfDebug unconditionally assumes for all call instructions the 0th
operand is the callee operand, which seems to be true for other targets,
but not for WebAssembly. This adds TargetInstrInfo::getCallOperand
method whose default implementation returns getOperand(0) and makes
WebAssembly overrides it to use its own utility method to get the callee
operand.

Edit:
This also fixes an existing bug in WebAssembly::getCalleeOp, which was
uncovered by this CL.

[aheejin]

llvm/test/CodeGen/WebAssembly/stackified-debug.ll

2

Without this patch this fails, because

%5:i32 = CALL @input, ...

In this wasm MIR instruction, the callee is the 1th operand, but the current code will incorrectly assume the 0th operand, which is the dest register (%5 here), as a callee, and will try to print its register name, when --debug is given. This crashes because wasm does not use physical registers.

[djtodoro]

Thanks!

llvm/test/CodeGen/WebAssembly/stackified-debug.ll

2

I'd prefer a dedicated (standalone) test for this.

[dschuff]

I think this makes sense.

my extra comment was that it seems likely that there are other places that currently use getOperand(0) that should now use this, and I was wondering if there were an easy way to find them since we can't just grep (especially since LLVM IR also has a getOperand). Thinking about that some more: Presumably uses in other backends would be safe to leave in place, and anything that affects codegen (at least in code that is covered at all) would be pretty broken, so maybe there actually aren't that many places outside of debug info (in other words broken debug info would be much more likely to go unnoticed than broken codegen).

[aheejin]

In D102978#2777793, @dschuff wrote:

I think this makes sense.

my extra comment was that it seems likely that there are other places that currently use getOperand(0) that should now use this, and I was wondering if there were an easy way to find them since we can't just grep (especially since LLVM IR also has a getOperand). Thinking about that some more: Presumably uses in other backends would be safe to leave in place, and anything that affects codegen (at least in code that is covered at all) would be pretty broken, so maybe there actually aren't that many places outside of debug info (in other words broken debug info would be much more likely to go unnoticed than broken codegen).

I tried to search words like CalleeOp in the codebase but all other usages are in target-specific code, and there are only a few of them anyway. Also I think in other places it would have pretty much caused crashes anyway, so I'm not worried too much about it.

[aheejin]

New standalone test

[aheejin]

Comment fix

llvm/test/CodeGen/WebAssembly/stackified-debug.ll

2

I added test/DebugInfo/WebAssembly/call-site.ll and deleted this --debug line. PTAL.

llvm/test/DebugInfo/WebAssembly/dbg-value-list.ll

36 ↗

(On Diff #347517)

This is just a drive-by name fix and unrelated to this CL

[aheejin]

llvm/test/DebugInfo/WebAssembly/call-site.ll

8 ↗

(On Diff #347518)

Without the fix in this CL, this "foo" is not correctly displayed.

[dschuff]

llvm/test/DebugInfo/WebAssembly/call-site.ll

8 ↗

(On Diff #347518)

Thanks, this is also great because I don't know if we have any other tests that specifically cover callsite info for wasm anyway.

[aheejin]

Fix WebAssembly::getCalleeOp for indirect calls

[aheejin]

This happened to uncover a long-existing bug in WebAssembly::getCalleeOp. For indirect calls it returns MI.getOperand(MI.getNumOperands() - 1), but in the presence of implicit operands, most of the time it returns an incorrect operand. For example:

%37:i32 = CALL_INDIRECT 0, 0, %55:i32, %38:i32, %54:i32, implicit-def dead $arguments, implicit $sp32, implicit $sp64, implicit-def dead $arguments, implicit $sp32, implicit $sp64, implicit-def $value_stack, implicit $value_stack, debug-location !39; system/lib/libc/musl/src/stdlib/bsearch.c:9:10

The callee operand is %54 here, but this returns implicit $value_stack. The should have used MI.getNumExplicitOperand() instead. This has not been a problem so far because this utility function has been used only from a few places and all of them only cared about direct calls.

This fixes this bug and adds a test for call_indirect too.

[djtodoro]

LGTM (with the comments addressed)

llvm/lib/Target/WebAssembly/Utils/WebAssemblyUtilities.cpp

94 ↗

(On Diff #347589)

By doing this, the patch does a bit more then described in the summary. I think this is should be a separate change.

llvm/test/DebugInfo/WebAssembly/call-site.ll

3 ↗

(On Diff #347589)

Super nit: The highlevel comments starts with double ;;

typo: information

36 ↗

(On Diff #347589)

super nit: producer could be "clang version 11.0.0" only

llvm/test/DebugInfo/WebAssembly/dbg-value-list.ll

36 ↗

(On Diff #347517)

This should be a separate change.

[aheejin]

llvm/lib/Target/WebAssembly/Utils/WebAssemblyUtilities.cpp

94 ↗

(On Diff #347589)

I thought about making it a separate change, but it is hard.

• If we land DwarfDebug fix first, it will break all programs with indirect calls and debug info without this getCalleeOp fix.
• If we land getCalleeOp fix first, there is no way to test that change because DwarfDebug is currently the only place we use getCalleeOp for indirect calls.

By the way, I fixed the summary to include this fix.

[aheejin]

Address comments

[djtodoro]

llvm/lib/Target/WebAssembly/Utils/WebAssemblyUtilities.cpp

94 ↗

(On Diff #347589)

OK, thanks

llvm/test/DebugInfo/WebAssembly/call-site.ll

23 ↗

(On Diff #347592)

why don't we have here an additional CHECK for the register that holds the indirect call (DW_AT_GNU_call_site_target)?

[aheejin]

llvm/test/DebugInfo/WebAssembly/call-site.ll

23 ↗

(On Diff #347592)

It doesn't have it. Am I doing something wrong? This is the full llvm-dwarfdump output for this file.

call-site.o:  file format WASM                                                   
                                                                                 
.debug_info contents:                                                            
0x00000000: Compile Unit: length = 0x00000068, format = DWARF32, version = 0x0004, abbr_offset = 0x0000, addr_size = 0x04 (next unit at 0x0000006c)
                                                                                 
0x0000000b: DW_TAG_compile_unit                                                  
              DW_AT_producer  ("clang version 11.0.0")                           
              DW_AT_language  (DW_LANG_C99)                                      
              DW_AT_name  ("test.c")                                             
              DW_AT_stmt_list  (0x00000000)                                      
              DW_AT_comp_dir  ("/home/llvm-project")                             
              DW_AT_GNU_pubnames  (true)                                         
              DW_AT_low_pc  (0x00000000)                                         
              DW_AT_ranges  (0x00000000                                          
                 [0x00000002, 0x0000000a)                                        
                 [0x0000000b, 0x0000001a))                                       
                                                                                 
0x00000026:   DW_TAG_subprogram                                                  
                DW_AT_low_pc  (0x00000002)                                       
                DW_AT_high_pc  (0x0000000a)                                      
                DW_AT_frame_base  (DW_OP_WASM_location 0x3 0x0, DW_OP_stack_value)
                DW_AT_GNU_all_call_sites  (true)                                 
                DW_AT_name  ("call_direct")                                      
                DW_AT_decl_file  ("/home/llvm-project/test.c")                   
                DW_AT_decl_line  (3)                                             
                DW_AT_external  (true)                                           
                                                                                 
0x0000003d:     DW_TAG_GNU_call_site                                             
                  DW_AT_abstract_origin  (0x00000047 "foo")                      
                  DW_AT_low_pc  (0x00000009)                                     
                                                                                 
0x00000046:     NULL                                                             
                                                                                 
0x00000047:   DW_TAG_subprogram                                                  
                DW_AT_name  ("foo")                                              
                DW_AT_decl_file  ("/home/llvm-project/test.c")                   
                DW_AT_decl_line  (30)                                            
                DW_AT_declaration  (true)                                        
                DW_AT_external  (true)                                           
                                                                                 
0x0000004e:   DW_TAG_subprogram                                                  
                DW_AT_low_pc  (0x0000000b)                                       
                DW_AT_high_pc  (0x0000001a)                                      
                DW_AT_frame_base  (DW_OP_WASM_location 0x3 0x0, DW_OP_stack_value)
                DW_AT_GNU_all_call_sites  (true)                                 
                DW_AT_name  ("call_indirect")                                    
                DW_AT_decl_file  ("/home/llvm-project/test.c")                   
                DW_AT_decl_line  (3)                                             
                DW_AT_external  (true)                                           
                                                                                 
0x00000065:     DW_TAG_GNU_call_site                                             
                  DW_AT_low_pc  (0x00000019)                                     
                                                                                 
0x0000006a:     NULL                                                             
                                                                                 
0x0000006b:   NULL

It only seems to contain

0x00000065:     DW_TAG_GNU_call_site                                             
                  DW_AT_low_pc  (0x00000019)

for the indirect call. I'm not very familiar with dwarf info stuff myself, so please let me know if something is missing. Thanks.

[djtodoro]

llvm/test/DebugInfo/WebAssembly/call-site.ll

23 ↗

(On Diff #347592)

Actually, this is useless call_site debug information, since it does not have the DW_AT_GNU_call_site_target attribute (which holds the location of the indirect call).
In this case the call instruction is:

%6:i32 = CALL_INDIRECT 0, 0, %5:i32, %4:i32, %7:i32, implicit-def dead $arguments, implicit $sp32, implicit $sp64, implicit-def dead $arguments, implicit $sp32, implicit $sp64, implicit-def $value_stack, implicit $value_stack, debug-location !11; test.c:40:11

And the callee op is %7:i32, which is not a physical register, so there is no way to generate MachineLocation for it. The WebAssembly indirect calls should be handled differently I guess.

For now, we can do:

const MachineOperand &CalleeOp = TII->getCalleeOperand(MI);
if (!CalleeOp.isGlobal() &&
    (!CalleeOp.isReg() || !Register::isPhysicalRegister(CalleeOp.getReg())))
   continue;

[aheejin]

Address comments

[aheejin]

Comment fix

[djtodoro]

Thanks!

llvm/include/llvm/CodeGen/TargetInstrInfo.h

public:
 
   /// Returns the target-specific default value for tail duplication.
   /// This value will be used if the tail-dup-placement-threshold argument is
   /// not provided.
   virtual unsigned getTailDuplicateSize(CodeGenOpt::Level OptLevel) const {
     return OptLevel >= CodeGenOpt::Aggressive ? 4 : 2;
   }
 
+  /// Returns the callee operand from the given \p MI.
+  virtual const MachineOperand &getCalleeOperand(const MachineInstr &MI) const {
+    return MI.getOperand(0);
+  }
+
 private:
   mutable std::unique_ptr<MIRFormatter> Formatter;
   unsigned CallFrameSetupOpcode, CallFrameDestroyOpcode;
   unsigned CatchRetOpcode;
   unsigned ReturnOpcode;
 };
 
 /// Provide DenseMapInfo for TargetInstrInfo::RegSubRegPair.

llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp

for (const MachineInstr &MI : MBB.instrs()) {
 
       // Check if delay slot support is enabled.
       if (MI.hasDelaySlot() && !delaySlotSupported(*&MI))
         return;
 
       // If this is a direct call, find the callee's subprogram.
       // In the case of an indirect call find the register that holds
       // the callee.
-      const MachineOperand &CalleeOp = MI.getOperand(0);
+      const MachineOperand &CalleeOp = TII->getCalleeOperand(MI);
       if (!CalleeOp.isGlobal() && !CalleeOp.isReg())
         continue;
 
       unsigned CallReg = 0;
       DIE *CalleeDIE = nullptr;
       const Function *CalleeDecl = nullptr;
       if (CalleeOp.isReg()) {
         CallReg = CalleeOp.getReg();

llvm/lib/Target/WebAssembly/WebAssemblyInstrInfo.h

unsigned insertBranch(MachineBasicBlock &MBB, MachineBasicBlock *TBB,
                         MachineBasicBlock *FBB, ArrayRef<MachineOperand> Cond,
                         const DebugLoc &DL,
                         int *BytesAdded = nullptr) const override;
   bool
   reverseBranchCondition(SmallVectorImpl<MachineOperand> &Cond) const override;
 
   ArrayRef<std::pair<int, const char *>>
   getSerializableTargetIndices() const override;
+
+  const MachineOperand &getCalleeOperand(const MachineInstr &MI) const override;
 };
 
 } // end namespace llvm
 
 #endif

llvm/lib/Target/WebAssembly/WebAssemblyInstrInfo.cpp

 //===-- WebAssemblyInstrInfo.cpp - WebAssembly Instruction Information ----===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 ///
 /// \file
 /// This file contains the WebAssembly implementation of the
 /// TargetInstrInfo class.
 ///
 //===----------------------------------------------------------------------===//
 
 #include "WebAssemblyInstrInfo.h"
 #include "MCTargetDesc/WebAssemblyMCTargetDesc.h"
+#include "Utils/WebAssemblyUtilities.h"
 #include "WebAssembly.h"
 #include "WebAssemblyMachineFunctionInfo.h"
 #include "WebAssemblySubtarget.h"
 #include "llvm/CodeGen/MachineFrameInfo.h"
 #include "llvm/CodeGen/MachineInstrBuilder.h"
 #include "llvm/CodeGen/MachineMemOperand.h"
 #include "llvm/CodeGen/MachineRegisterInfo.h"
 using namespace llvm;
WebAssemblyInstrInfo::getSerializableTargetIndices() const {
   static const std::pair<int, const char *> TargetIndices[] = {
       {WebAssembly::TI_LOCAL, "wasm-local"},
       {WebAssembly::TI_GLOBAL_FIXED, "wasm-global-fixed"},
       {WebAssembly::TI_OPERAND_STACK, "wasm-operand-stack"},
       {WebAssembly::TI_GLOBAL_RELOC, "wasm-global-reloc"},
       {WebAssembly::TI_LOCAL_INDIRECT, "wasm-local-indirect"}};
   return makeArrayRef(TargetIndices);
 }
+
+const MachineOperand &
+WebAssemblyInstrInfo::getCalleeOperand(const MachineInstr &MI) const {
+  return WebAssembly::getCalleeOp(MI);
+}

llvm/test/CodeGen/WebAssembly/stackified-debug.ll

 ; RUN: llc -verify-machineinstrs < %s | FileCheck %s
+; RUN: llc --debug < %s
 ; RUN: llc -filetype=obj %s -o - | llvm-dwarfdump - | FileCheck %s --check-prefix DWARF
 
 ; Input C code:
 
 ;  int i = input();  // Nested case
 ;  int j = input();  // Trivial def-use.
 ;  output(i, j);