[PATCH] D44356: [LICM] Salvage DebugInfo from dying Instructions

Vedant Kumar via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Mar 12 16:07:32 PDT 2018


vsk added a comment.

Thanks for working on this! The change in LICM looks good, but the tests need a little bit of work.



================
Comment at: test/Transforms/LICM/2011-04-09-RAUW-AST.ll:2
 ; RUN: opt < %s -loop-rotate -licm -S | FileCheck %s
+; RUN: opt < %s -debugify -loop-rotate -licm -S | FileCheck %s -check-prefix=DEBUGIFY
 ; PR9604
----------------
Why does this test need to be updated in addition to sinking.ll? It seems like one test case should be enough to exercise the new functionality.


================
Comment at: test/Transforms/LICM/sinking.ll:314
 ; CHECK-NEXT:    %l.le = trunc i64 %[[LCSSAPHI]] to i32
-; CHECK-NEXT:    ret i32 %l.le
+; DEBUGIFY:    call void @llvm.dbg.value(metadata i32 %l.le, metadata !143, metadata !DIExpression())
+; CHECK:    ret i32 %l.le
----------------
Generally we try not to hardcode metadata numbers into CHECK lines, because it can make for a brittle test case. We wouldn't want this test to fail if e.g the order in which metadata are emitted changes.

Instead of "!143", writing "{{.*}}" here would be fine.


================
Comment at: test/Transforms/LICM/sinking.ll:315
+; DEBUGIFY:    call void @llvm.dbg.value(metadata i32 %l.le, metadata !143, metadata !DIExpression())
+; CHECK:    ret i32 %l.le
 
----------------
This used to be a CHECK-NEXT, but now it's a CHECK. That loosens the test, as FileCheck will no longer require that the "ret i32" occur immediately after the "trunc i64" instruction.

Can you move the CHECK-NEXT back to where it was, and add the DEBUGIFY check line after it?


Repository:
  rL LLVM

https://reviews.llvm.org/D44356





More information about the llvm-commits mailing list