[PATCH] D102978: [WebAssembly] Add TargetInstrInfo::getCalleeOperand
Djordje Todorovic via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon May 24 23:52:12 PDT 2021
djtodoro added a comment.
LGTM (with the comments addressed)
================
Comment at: llvm/lib/Target/WebAssembly/Utils/WebAssemblyUtilities.cpp:94
case WebAssembly::RET_CALL_INDIRECT_S:
- return MI.getOperand(MI.getNumOperands() - 1);
+ return MI.getOperand(MI.getNumExplicitOperands() - 1);
default:
----------------
By doing this, the patch does a bit more then described in the summary. I think this is should be a separate change.
================
Comment at: llvm/test/DebugInfo/WebAssembly/call-site.ll:3
+
+; This checks if the call site informstion is correctly written in debug info.
+; This is a regression test for the bug that DwarfDebug unconditionally assumed
----------------
Super nit: The highlevel comments starts with double `;;`
typo: information
================
Comment at: llvm/test/DebugInfo/WebAssembly/call-site.ll:36
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang version 11.0.0 (https://github.com/llvm/llvm-project.git ed7aaf832444411ce93aa0443425ce401f5c7a8e)", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug)
+!1 = !DIFile(filename: "test.c", directory: "/home/llvm-project")
----------------
super nit: producer could be "clang version 11.0.0" only
================
Comment at: llvm/test/DebugInfo/WebAssembly/dbg-value-list.ll:36
!5 = !{i32 1, !"wchar_size", i32 4}
-!6 = distinct !DISubprogram(name: "", scope: !1, file: !1, line: 3, type: !7, scopeLine: 3, flags: DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0)
+!6 = distinct !DISubprogram(name: "dbg_value_list_test", scope: !1, file: !1, line: 3, type: !7, scopeLine: 3, flags: DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0)
!7 = !DISubroutineType(types: !{null})
----------------
aheejin wrote:
> This is just a drive-by name fix and unrelated to this CL
This should be a separate change.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D102978/new/
https://reviews.llvm.org/D102978
More information about the llvm-commits
mailing list