[spirv-opt] debug info preservation in ssa-rewrite by jaebaek · Pull Request #3356 · KhronosGroup/SPIRV-Tools

jaebaek · 2020-05-19T15:25:32Z

OpenCL.DebugInfo.100 spec mimics
llvm.dbg.declare and llvm.dbg.value of LLVM IR
using DebugDeclare and DebugValue.

For example, we can describe an OpVariable for a local
variable using DebugDeclare and when it is updated by
OpStore we can specify the new value for the local variable
using DebugValue.

As what LLVM ssa-rewrite does, we want to add
DebugValue for variable updates and phi instructions that
will be used by a shader debugger. This commit lets
ssa-rewrite pass add DebugValue instructions for store
and a phi instructions to specify a result id of a value is
corresponding to the value of a local variable. Note that
ssa-rewrite will remove DebugDeclare after adding all
DebugValue instructions properly.

jaebaek · 2020-05-19T15:32:32Z

As far as I understand, the only thing we must do is updating operands of DebugDeclare and DebugValue according to the load/store/variable updates in the ssa-rewrite.

Actually, since bool IRContext::ReplaceAllUsesWith(uint32_t before, uint32_t after) does the replacement correctly, the only thing we have to do is to include result ids in the targets if they are used by DebugDeclare and DebugValue in addition to OpLoad, OpStore, and OpVariable.

I will add more test cases later but I first want to doublecheck the idea before taking long time for writing test cases.

s-perron · 2020-05-19T15:58:55Z

This does not seem right. The ssa rewriter needs to create new DebugValue instructions every time it removes a store. We can talk offline, but the simple example we should walk through is

#version 140
  
in vec4 BC;
out float fo;
  
void main()
{
  float f = 0.0;
  if (BD.x > 0.0) {
    f = 1.0;
  }
  fo = f;
}

jaebaek · 2020-05-25T22:14:39Z

I updated the code based on the offline discussion. PTAL!
I did not add many tests yet. Once we both agree the direction, I will add more tests.

s-perron · 2020-05-26T13:21:40Z

Two general comments. First, remember that the analysis should represent information that can be rebuild from the spir-v, and nothing more. It should not contain any state specific to a particular pass. Something like this is not right because linking a debug value instruction to a particular load or store is not possible when looking at the spir-v after the pass.

Second, it does not look like you are adding a debug value after an OpPhi is generated. The DebugValue instructions should be use to say which expression can be used to get the value for a given variable. That expression must be valid if the debugger is going to work. Loads are unimportant because they do not affect the value of the variable. Stores are important because they change the value of a variable. Merge blocks are important because the old value may not be accurate anymore.
If a OpPhi is added for that variable, then you need a new DebugValue. If you want to see how this is suppose to work look at clang.

If the cpp file is

int foo(bool b)                                   
{ 
  int i = 0;
  if (b) {
    i = foo(false); 
  }
  if (!b) {
    i = 1;
  }                                                                                                                                 
  return i; 
}

Compile using clang++ t.cpp -g -O -c -mllvm -print-after-all. Notice that when the debug value instructions are added, there is a debug value after every phi instruction.

jaebaek · 2020-05-27T19:38:59Z

Now it adds DebugValue only for applicable OpStore and OpPhi. PTAL.

s-perron

This is looking pretty good. It think the high level design is good. I have a couple questions about some details.

s-perron · 2020-06-02T14:11:09Z

test/opt/local_ssa_elim_test.cpp

+; CHECK:      OpExtInst %void [[ext]] DebugDeclare [[dbg_f]] %f
+; CHECK:      OpExtInst %void [[ext]] DebugDeclare [[dbg_i]] %i
+; CHECK:      [[phi0:%\w+]] = OpPhi %float %float_0
+; CHECK-NEXT: OpExtInst %void [[ext]] DebugValue [[dbg_f]] [[phi0]]


Can you have debug value instruction mixed in with the OpPhi instructions?

Yes. Are there something you are worried about?

I do not think the following code has any issue:

%a = OpPhi ... DebugValue %dbg_a %a %b = OpPhi ... DebugValue %dbg_b %b %c = OpPhi ... DebugValue %dbg_c %c ...

The current spirv-val does not report any error/warning.

The spec says:

Within a block, this instruction must appear before all non-OpPhi instructions (except for OpLine and OpNoLine, which can be mixed with OpPhi).

I do not see anything in the debug instruction spec that changes this. Personally, it does not make sense that a debug value or debug declare should be mixed with the OpPhi instructions. Technically the spec says they are not allowed (regardless of what spirv-val does).

We should double check this.

Oh I see. Now I checked it in the description of OpPhi. I fixed it.

s-perron · 2020-06-02T14:13:04Z

test/opt/local_ssa_elim_test.cpp

+; CHECK:      OpStore %f [[f_val:%\w+]]
+; CHECK-NEXT: OpExtInst %void [[ext]] DebugValue [[dbg_f]] [[f_val]]
+; CHECK:      OpStore %i [[i_val:%\w+]]
+; CHECK-NEXT: OpExtInst %void [[ext]] DebugValue [[dbg_i]] [[i_val]]


It might be good to provide some context by checking that these are all in the loop.

s-perron · 2020-06-02T14:15:18Z

test/opt/local_ssa_elim_test.cpp

+; CHECK:      OpStore %f %float_0
+; CHECK:      OpStore %i %int_0
+; CHECK:      OpExtInst %void [[ext]] DebugDeclare [[dbg_f]] %f
+; CHECK:      OpExtInst %void [[ext]] DebugDeclare [[dbg_i]] %i


Is there a DebugValue after the initialization of f and i? If not, there should be. Otherwise, we will have to keep the variable around otherwise the debugger won't know what the value is until it reaches the first debug value.

I'm wondering if we should be keeping the debug delcare around if the ssa rewriter removes the use of the variable. Do you know what happens in LLVM?

For the first question, yes it added DebugValue right after the initialization. I will add it in this unit test.
Before:

%main = OpFunction %void None %8 %22 = OpLabel %f = OpVariable %_ptr_Function_float Function %i = OpVariable %_ptr_Function_int Function OpStore %f %float_0 OpStore %i %int_0 %decl0 = OpExtInst %void %ext DebugDeclare %dbg_f %f %null_expr %decl1 = OpExtInst %void %ext DebugDeclare %dbg_i %i %null_expr OpBranch %23 ... (other code) ...

After ssa-rewrite:

%main = OpFunction %void None %14 %39 = OpLabel %f = OpVariable %_ptr_Function_float Function %i = OpVariable %_ptr_Function_int Function OpStore %f %float_0 %57 = OpExtInst %void %2 DebugValue %37 %float_0 %30 OpStore %i %int_0 %58 = OpExtInst %void %2 DebugValue %38 %int_0 %30 %40 = OpExtInst %void %2 DebugDeclare %37 %f %30 %41 = OpExtInst %void %2 DebugDeclare %38 %i %30 OpBranch %42 ... (other code) ...

In terms of the second question, it seems that LLVM removes llvm.dbg.declare after ssa-rewrite. I did not remove DebugDeclare. Thank you for pointing it. I will update it.

before:

1 ; Function Attrs: uwtable 2 define dso_local i32 @_Z3foob(i1 zeroext) #0 !dbg !7 { 3 %2 = alloca i8, align 1 4 %3 = alloca i32, align 4 5 %4 = zext i1 %0 to i8 6 store i8 %4, i8* %2, align 1, !tbaa !15 7 call void @llvm.dbg.declare(metadata i8* %2, metadata !13, metadata !DIExpression()), !dbg !19 8 %5 = bitcast i32* %3 to i8*, !dbg !20 9 call void @llvm.lifetime.start.p0i8(i64 4, i8* %5) #3, !dbg !20 10 call void @llvm.dbg.declare(metadata i32* %3, metadata !14, metadata !DIExpression()), !dbg !21 11 store i32 0, i32* %3, align 4, !dbg !21, !tbaa !22 12 %6 = load i8, i8* %2, align 1, !dbg !24, !tbaa !15, !range !26 13 %7 = trunc i8 %6 to i1, !dbg !24 14 br i1 %7, label %8, label %10, !dbg !27 ...

after:

1 ; Function Attrs: uwtable 2 define dso_local i32 @_Z3foob(i1 zeroext) #0 !dbg !7 { 3 %2 = zext i1 %0 to i8 4 call void @llvm.dbg.value(metadata i8 %2, metadata !13, metadata !DIExpression()), !dbg !15 5 call void @llvm.dbg.value(metadata i32 0, metadata !14, metadata !DIExpression()), !dbg !15 6 %3 = trunc i8 %2 to i1, !dbg !16 7 br i1 %3, label %4, label %6, !dbg !18 ...

See %2.

LLVM gets rid of the alloca as well, which is a different design decision that what we did. Diego decided to keep the stores, so the OpVariable instruction must stick around.

I'm leaning towards removing the debug declare in ssa-rewrite, but we should have a quick discussion about it tomorrow with Diego.

s-perron · 2020-06-02T14:18:38Z

test/opt/local_ssa_elim_test.cpp

+  SinglePassRunAndMatch<SSARewritePass>(text, true);
+}
+
+TEST_F(LocalSSAElimTest, DebugLostCopyProblem) {


Can you add a comment explaining what the lost copy problem is? It does not mean much to me.

I added this because the LostCopyProblem test case exists. However, it does not mean much to me as well. Let me remove it.

s-perron · 2020-06-02T14:19:22Z

test/opt/local_ssa_elim_test.cpp

+  SinglePassRunAndMatch<SSARewritePass>(text, true);
+}
+
+TEST_F(LocalSSAElimTest, DebugSwapProblem) {


Same as before. I don't know what that problem is.

I added a comment. When I checked this case, I saw the following two phi instructions

%65 = OpPhi %float %float_1 %39 %64 %47 %64 = OpPhi %float %float_0 %39 %65 %47

They are operands of each other i.e., %64 and %65. I want to check we correctly add DebugValue for those phi instructions of %f1 and %f2.

jaebaek

Thank you for your code review. I updated the code based on it. PTAL!

jaebaek · 2020-06-02T16:21:08Z

test/opt/local_ssa_elim_test.cpp

+; CHECK:      OpStore %f %float_0
+; CHECK:      OpStore %i %int_0
+; CHECK:      OpExtInst %void [[ext]] DebugDeclare [[dbg_f]] %f
+; CHECK:      OpExtInst %void [[ext]] DebugDeclare [[dbg_i]] %i


For the first question, yes it added DebugValue right after the initialization. I will add it in this unit test.
Before:

%main = OpFunction %void None %8 %22 = OpLabel %f = OpVariable %_ptr_Function_float Function %i = OpVariable %_ptr_Function_int Function OpStore %f %float_0 OpStore %i %int_0 %decl0 = OpExtInst %void %ext DebugDeclare %dbg_f %f %null_expr %decl1 = OpExtInst %void %ext DebugDeclare %dbg_i %i %null_expr OpBranch %23 ... (other code) ...

After ssa-rewrite:

%main = OpFunction %void None %14 %39 = OpLabel %f = OpVariable %_ptr_Function_float Function %i = OpVariable %_ptr_Function_int Function OpStore %f %float_0 %57 = OpExtInst %void %2 DebugValue %37 %float_0 %30 OpStore %i %int_0 %58 = OpExtInst %void %2 DebugValue %38 %int_0 %30 %40 = OpExtInst %void %2 DebugDeclare %37 %f %30 %41 = OpExtInst %void %2 DebugDeclare %38 %i %30 OpBranch %42 ... (other code) ...

In terms of the second question, it seems that LLVM removes llvm.dbg.declare after ssa-rewrite. I did not remove DebugDeclare. Thank you for pointing it. I will update it.

before:

1 ; Function Attrs: uwtable 2 define dso_local i32 @_Z3foob(i1 zeroext) #0 !dbg !7 { 3 %2 = alloca i8, align 1 4 %3 = alloca i32, align 4 5 %4 = zext i1 %0 to i8 6 store i8 %4, i8* %2, align 1, !tbaa !15 7 call void @llvm.dbg.declare(metadata i8* %2, metadata !13, metadata !DIExpression()), !dbg !19 8 %5 = bitcast i32* %3 to i8*, !dbg !20 9 call void @llvm.lifetime.start.p0i8(i64 4, i8* %5) #3, !dbg !20 10 call void @llvm.dbg.declare(metadata i32* %3, metadata !14, metadata !DIExpression()), !dbg !21 11 store i32 0, i32* %3, align 4, !dbg !21, !tbaa !22 12 %6 = load i8, i8* %2, align 1, !dbg !24, !tbaa !15, !range !26 13 %7 = trunc i8 %6 to i1, !dbg !24 14 br i1 %7, label %8, label %10, !dbg !27 ...

after:

1 ; Function Attrs: uwtable 2 define dso_local i32 @_Z3foob(i1 zeroext) #0 !dbg !7 { 3 %2 = zext i1 %0 to i8 4 call void @llvm.dbg.value(metadata i8 %2, metadata !13, metadata !DIExpression()), !dbg !15 5 call void @llvm.dbg.value(metadata i32 0, metadata !14, metadata !DIExpression()), !dbg !15 6 %3 = trunc i8 %2 to i1, !dbg !16 7 br i1 %3, label %4, label %6, !dbg !18 ...

See %2.

jaebaek · 2020-06-02T16:25:00Z

test/opt/local_ssa_elim_test.cpp

+; CHECK:      OpExtInst %void [[ext]] DebugDeclare [[dbg_f]] %f
+; CHECK:      OpExtInst %void [[ext]] DebugDeclare [[dbg_i]] %i
+; CHECK:      [[phi0:%\w+]] = OpPhi %float %float_0
+; CHECK-NEXT: OpExtInst %void [[ext]] DebugValue [[dbg_f]] [[phi0]]


Yes. Are there something you are worried about?

I do not think the following code has any issue:

%a = OpPhi ... DebugValue %dbg_a %a %b = OpPhi ... DebugValue %dbg_b %b %c = OpPhi ... DebugValue %dbg_c %c ...

The current spirv-val does not report any error/warning.

jaebaek · 2020-06-02T18:30:10Z

test/opt/local_ssa_elim_test.cpp

+; CHECK:      OpStore %f [[f_val:%\w+]]
+; CHECK-NEXT: OpExtInst %void [[ext]] DebugValue [[dbg_f]] [[f_val]]
+; CHECK:      OpStore %i [[i_val:%\w+]]
+; CHECK-NEXT: OpExtInst %void [[ext]] DebugValue [[dbg_i]] [[i_val]]


jaebaek · 2020-06-02T18:51:01Z

test/opt/local_ssa_elim_test.cpp

+  SinglePassRunAndMatch<SSARewritePass>(text, true);
+}
+
+TEST_F(LocalSSAElimTest, DebugLostCopyProblem) {


I added this because the LostCopyProblem test case exists. However, it does not mean much to me as well. Let me remove it.

jaebaek · 2020-06-02T18:54:45Z

test/opt/local_ssa_elim_test.cpp

+  SinglePassRunAndMatch<SSARewritePass>(text, true);
+}
+
+TEST_F(LocalSSAElimTest, DebugSwapProblem) {


I added a comment. When I checked this case, I saw the following two phi instructions

%65 = OpPhi %float %float_1 %39 %64 %47 %64 = OpPhi %float %float_0 %39 %65 %47

They are operands of each other i.e., %64 and %65. I want to check we correctly add DebugValue for those phi instructions of %f1 and %f2.

dnovillo · 2020-06-04T13:08:15Z

No description provided.

Please provide a description for this change.

s-perron

Looking good. Just a few small changes needed.

s-perron · 2020-06-17T18:59:35Z

source/opt/debug_info_manager.cpp

+
+  uint32_t instr_scope_id = instr->GetDebugScope().GetLexicalScope();
+  for (auto* dbg_decl_or_val : dbg_decl_itr->second) {
+    if (!IsDeclareVisibleToInstr(dbg_decl_or_val, instr_scope_id)) continue;


You are assuming that the debug value has to come next to the instruction that is in the scope with which it will belong. We should take the scope as a parameter.

It's a little confusing. I added one more check IsVariableVisibleToInstr() right after IsDeclareVisibleToInstr(), which returns true if the variable is visible in the scope of the instruction. Please let me know if I understand your intention incorrectly.

That is not what I meant. I meant that you use |instr| for two purposes:

Identify the position to place the new instructions: The first legal position after |intsr|.

As the debug scope of the new instruction.

I don't think that they would have to be the same. How hard it is to split these two things into two separate parameters: Instruction* insert_pos, uint32_t debug_scope_id.

Remove IsVariableVisibleToInstr. That is not useful. The debug scope of the instruction allocating the memory (OpVariable) is irrelevant. You had it right the first time.

Remove IsVariableVisibleToInstr. That is not useful. The debug scope of the instruction allocating the memory (OpVariable) is irrelevant. You had it right the first time.

I agree.
It makes sense. I updated it.

s-perron · 2020-06-17T19:00:57Z

source/opt/debug_info_manager.h

                                    DebugInlinedAtContext* inlined_at_ctx);

+  // Creates a DebugValue whose 'Local Variable' and 'Value' operands are
+  // |variable_id| and |value_id| and inserts it after |instr|.


This needs to be updated. It might create more than one, and is not clear on what the function is doing.

Try something like:

Generates a DebugValue instruction with value |value_id| for every local variable that is in scope and whose memory is |variable_id|.

s-perron · 2020-06-17T19:06:54Z

source/opt/debug_info_manager.cpp

+  if (instr_scope_id == kNoDebugScope) return false;
+
+  uint32_t dbg_local_var_id =
+      dbg_declare->GetSingleWordOperand(kDebugValueOperandLocalVariableIndex);


Is this a debug declare or debugValue? You seem to mix the two.

Changed it to kDebugDeclareOperandLocalVariableIndex.

jaebaek

Thank you for your code review.
I am not sure I correctly understand the scope check for the variable when it adds DebugValue.
PTAL!

jaebaek · 2020-06-17T21:08:59Z

source/opt/debug_info_manager.cpp

+  if (instr_scope_id == kNoDebugScope) return false;
+
+  uint32_t dbg_local_var_id =
+      dbg_declare->GetSingleWordOperand(kDebugValueOperandLocalVariableIndex);


Changed it to kDebugDeclareOperandLocalVariableIndex.

jaebaek · 2020-06-17T21:11:24Z

source/opt/debug_info_manager.cpp

+
+  uint32_t instr_scope_id = instr->GetDebugScope().GetLexicalScope();
+  for (auto* dbg_decl_or_val : dbg_decl_itr->second) {
+    if (!IsDeclareVisibleToInstr(dbg_decl_or_val, instr_scope_id)) continue;


It's a little confusing. I added one more check IsVariableVisibleToInstr() right after IsDeclareVisibleToInstr(), which returns true if the variable is visible in the scope of the instruction. Please let me know if I understand your intention incorrectly.

jaebaek · 2020-06-17T21:19:09Z

source/opt/debug_info_manager.h

                                    DebugInlinedAtContext* inlined_at_ctx);

+  // Creates a DebugValue whose 'Local Variable' and 'Value' operands are
+  // |variable_id| and |value_id| and inserts it after |instr|.


s-perron · 2020-06-18T19:06:03Z

source/opt/debug_info_manager.cpp

+
+  uint32_t instr_scope_id = instr->GetDebugScope().GetLexicalScope();
+  for (auto* dbg_decl_or_val : dbg_decl_itr->second) {
+    if (!IsDeclareVisibleToInstr(dbg_decl_or_val, instr_scope_id)) continue;


That is not what I meant. I meant that you use |instr| for two purposes:

Identify the position to place the new instructions: The first legal position after |intsr|.

As the debug scope of the new instruction.

I don't think that they would have to be the same. How hard it is to split these two things into two separate parameters: Instruction* insert_pos, uint32_t debug_scope_id.

Remove IsVariableVisibleToInstr. That is not useful. The debug scope of the instruction allocating the memory (OpVariable) is irrelevant. You had it right the first time.

jaebaek

Thanks. Updated it based on your comment. PTAL!

jaebaek · 2020-06-18T20:45:24Z

source/opt/debug_info_manager.cpp

+
+  uint32_t instr_scope_id = instr->GetDebugScope().GetLexicalScope();
+  for (auto* dbg_decl_or_val : dbg_decl_itr->second) {
+    if (!IsDeclareVisibleToInstr(dbg_decl_or_val, instr_scope_id)) continue;


Remove IsVariableVisibleToInstr. That is not useful. The debug scope of the instruction allocating the memory (OpVariable) is irrelevant. You had it right the first time.

I agree.
It makes sense. I updated it.

Roll third_party/glslang/ ebf55a0..8397044 (17 commits) KhronosGroup/glslang@ebf55a0...8397044 $ git log ebf55a0..8397044 --date=short --no-merges --format='%ad %ae %s' 2020-06-22 gleese Update test expected files with new magic number 2020-06-22 gleese Update SPIR-V generator version 2020-06-05 gleese Update test results to expect OpFUnordNotEqual 2020-06-05 gleese Use OpFUnordNotEqual for floating-point != 2020-06-22 johnkslang Update README.md 2020-06-19 bclayton Add kokoro configs for android-ndk and cmake 2020-06-19 bclayton Switch ndk_test from gnustl_static to c++_static 2020-06-17 ShabbyX Add -g0 command line argument 2020-06-16 cepheus Build: use better MSVC subfolder names for the previous build changes. 2020-06-16 cepheus Bump version numbers. 2020-06-16 bclayton Move hlsl/ source to glslang/HLSL/ 2020-06-16 cepheus Bump version. 2020-06-15 bclayton CMake: Fold HLSL source into glslang 2020-06-15 dj2 Remove unused variable. (KhronosGroup#2273) 2020-06-15 rharrison Remove unused function, BaseTypeName (KhronosGroup#2272) 2020-06-15 cepheus HLSL: Remove support for having GLSL versions of HLSL intrinsics. 2020-06-10 bclayton C Interface: Split SPIR-V C interface to own file Roll third_party/googletest/ 8567b0929..c6e309b26 (2 commits) google/googletest@8567b09...c6e309b $ git log 8567b0929..c6e309b26 --date=short --no-merges --format='%ad %ae %s' 2020-06-17 absl-team Googletest export 2020-06-15 absl-team Googletest export Roll third_party/re2/ e9d517989..14d319322 (5 commits) google/re2@e9d5179...14d3193 $ git log e9d517989..14d319322 --date=short --no-merges --format='%ad %ae %s' 2020-06-18 junyer Write tests for the move semantics. 2020-06-18 junyer Improve RE2::Set and FilteredRE2 move semantics. 2020-06-16 junyer Distinguish between missing ')' and unexpected ')'. 2020-06-16 junyer Make RE2::Set and FilteredRE2 movable. 2020-06-15 junyer Herp derp. It's actually constant-time append. Roll third_party/spirv-cross/ 9e3df69d4..f9ae06512 (12 commits) KhronosGroup/SPIRV-Cross@9e3df69...f9ae065 $ git log 9e3df69d4..f9ae06512 --date=short --no-merges --format='%ad %ae %s' 2020-06-22 dsinclair Roll deps and update tests. 2020-06-22 post MSL: Remove the old VertexAttr API. 2020-06-19 cwallez Fix placement of SPIRV_CROSS_DEPRECATED. 2020-06-19 post Fix duplicated initialization for loop variables with initializers. 2020-06-18 post MSL: Add test case for constructing struct with non-value-type array. 2020-06-18 post MSL: Deal with loading non-value-type arrays. 2020-06-18 post MSL: Add tests for array copies in and out of buffers. 2020-06-18 post MSL: Improve handling of array types in buffer objects. 2020-06-18 post Clean up some deprecation warnings when building with Makefile. 2020-06-18 post Remove unused member in MSLShaderInput. 2020-06-13 cdavis MSL: Fix up input variables' vector lengths in all stages. 2020-06-16 post HLSL: Fix texProj in legacy HLSL. Roll third_party/spirv-tools/ 30bf46d..d4b9f57 (12 commits) KhronosGroup/SPIRV-Tools@30bf46d...d4b9f57 $ git log 30bf46d..d4b9f57 --date=short --no-merges --format='%ad %ae %s' 2020-06-19 jaebaek [spirv-opt] debug info preservation in ssa-rewrite (KhronosGroup#3356) 2020-06-19 ehsannas Updated desc_sroa to support flattening structures (KhronosGroup#3448) 2020-06-19 vasniktel spirv-fuzz: Refactor variable creation (KhronosGroup#3414) 2020-06-19 vasniktel spirv-fuzz: Swap operands in OpBranchConditional (KhronosGroup#3423) 2020-06-18 stevenperron Use structured order to unroll loops. (KhronosGroup#3443) 2020-06-18 jaebaek Debug info preservation in dead branch elimination (KhronosGroup#3425) 2020-06-17 vasniktel Add RemoveParameter method (KhronosGroup#3437) 2020-06-17 vasniktel Fix return type (KhronosGroup#3435) 2020-06-16 ehsannas Eliminate branches with condition of OpConstantNull (KhronosGroup#3438) 2020-06-16 andreperezmaselco.developer spirv-fuzz: Implement vector shuffle fuzzer pass (KhronosGroup#3412) 2020-06-16 andreperezmaselco.developer spirv-fuzz: Add replace linear algebra instruction transformation (KhronosGroup#3402) 2020-06-15 dj2 Update access control lists. (KhronosGroup#3433)

jaebaek added the component:optimization label May 19, 2020

jaebaek requested review from dnovillo and s-perron May 19, 2020 15:25

jaebaek self-assigned this May 19, 2020

jaebaek force-pushed the dbg_ssa_rewriter branch from 39bee85 to 16ef494 Compare May 25, 2020 22:13

jaebaek mentioned this pull request Jun 1, 2020

OpenCL.DebugInfo.100 information preservation in spirv-opt legalization passes #3386

Closed

19 tasks

s-perron requested changes Jun 2, 2020

View reviewed changes

jaebaek commented Jun 2, 2020

View reviewed changes

jaebaek force-pushed the dbg_ssa_rewriter branch from 003aca2 to dbc2484 Compare June 4, 2020 16:15

jaebaek force-pushed the dbg_ssa_rewriter branch from 2f49584 to ed1b609 Compare June 16, 2020 19:16

s-perron requested changes Jun 17, 2020

View reviewed changes

jaebaek commented Jun 17, 2020

View reviewed changes

s-perron requested changes Jun 18, 2020

View reviewed changes

jaebaek commented Jun 18, 2020

View reviewed changes

jaebaek added 9 commits June 19, 2020 10:44

[spirv-opt] debug info preservation in ssa-rewrite

bb3f313

Add DebugValue only for OpStore and OpPhi

0ec920f

Update based on code review

9ebf511

Do not mix DebugValue within phi instructions

b5ae9d3

Handle no DebugDeclare exists but we can use DebugValue for declaration

82ffe0d

Fix test failure

9b31ec9

Handle DebugDeclare for reference variables

096bea9

Updated based on code review

1b521de

Update based on code review

2711df4

jaebaek force-pushed the dbg_ssa_rewriter branch from 8c254ad to 2711df4 Compare June 19, 2020 14:47

s-perron approved these changes Jun 19, 2020

View reviewed changes

jaebaek added the kokoro:run label Jun 19, 2020

kokoro-team removed the kokoro:run label Jun 19, 2020

jaebaek merged commit d4b9f57 into KhronosGroup:master Jun 19, 2020

jaebaek deleted the dbg_ssa_rewriter branch June 19, 2020 18:57

jaebaek mentioned this pull request Jul 10, 2020

Remove DebugDeclare only for target variables in ssa-rewrite #3511

Merged

Conversation

jaebaek commented May 19, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jaebaek commented May 19, 2020

Uh oh!

s-perron commented May 19, 2020

Uh oh!

jaebaek commented May 25, 2020

Uh oh!

s-perron commented May 26, 2020

Uh oh!

jaebaek commented May 27, 2020

Uh oh!

s-perron left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jaebaek left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dnovillo commented Jun 4, 2020

Uh oh!

s-perron left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jaebaek left a comment

jaebaek commented May 19, 2020 •

edited

Loading