Skip to content

Commit

Permalink
Fix bug with masked wide llvm_gen_closure() (#1637)
Browse files Browse the repository at this point in the history
Signed-off-by: Daniel Dresser <danield@image-engine.com>
  • Loading branch information
danieldresser-ie committed Jan 25, 2023
1 parent 317c59e commit 5dad507
Show file tree
Hide file tree
Showing 6 changed files with 39 additions and 7 deletions.
2 changes: 1 addition & 1 deletion src/cmake/testing.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,7 @@ macro (osl_add_all_tests)
bug-array-heapoffsets bug-locallifetime bug-outputinit
bug-param-duplicate bug-peep bug-return
calculatenormal-reg
cellnoise closure closure-array closure-parameters closure-zero
cellnoise closure closure-array closure-parameters closure-zero closure-conditional
color color-reg colorspace comparison
complement-reg compile-buffer compassign-reg
component-range
Expand Down
15 changes: 9 additions & 6 deletions src/liboslexec/batched_llvm_gen.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7217,6 +7217,7 @@ LLVMGEN(llvm_gen_closure)
rop.ll.type_ptr(rop.llvm_type_closure_component_wide_ptr()));
llvm::Value* comp_wide_ptr = rop.ll.op_load(comp_wide_ptr_ptr);

llvm::Value* mask = rop.ll.current_mask();
// This is an unrolled vector-width loop. It was unrolled because it's
// was more expedient to write the for in C++ than IR, but if compilation
// of the extra ops becomes problematic, this choice can be revisited
Expand All @@ -7230,14 +7231,16 @@ LLVMGEN(llvm_gen_closure)
// For the weighted closures, we need a surrounding "if" so that it's safe
// for osl_allocate_weighted_closure_component to return NULL (unless we
// know for sure that it's constant weighted and that the weight is
// not zero).
// not zero). We also need the surrounding "if" to reject writing
// to closures that aren't masked.
llvm::BasicBlock* next_block = NULL;
llvm::BasicBlock* notnull_block = rop.ll.new_basic_block(
"non_null_closure");
next_block = rop.ll.new_basic_block("");
llvm::Value* cond
= rop.ll.op_ne(rop.ll.ptr_cast(comp_ptr, rop.ll.type_void_ptr()),
rop.ll.void_ptr_null());
"non_null_and_masked_closure");
next_block = rop.ll.new_basic_block("");
llvm::Value* cond = rop.ll.op_and(
rop.ll.op_ne(rop.ll.ptr_cast(comp_ptr, rop.ll.type_void_ptr()),
rop.ll.void_ptr_null()),
rop.ll.test_mask_lane(mask, lane_index));
rop.ll.op_branch(cond, notnull_block, next_block);

// If the closure has a "prepare" method, call
Expand Down
Empty file.
7 changes: 7 additions & 0 deletions testsuite/closure-conditional/ref/out.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
Compiled test.osl -> test.oso
Ci = (1, 0, 0) * emission ()
Ci = (1, 0, 0) * emission ()
Ci = (1, 0, 0) * emission ()
Ci = (0.75, 0.75, 0.75) * emission ()
Ci = (1, 1, 1) * emission ()

7 changes: 7 additions & 0 deletions testsuite/closure-conditional/run.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
#!/usr/bin/env python

# Copyright Contributors to the Open Shading Language project.
# SPDX-License-Identifier: BSD-3-Clause
# https://github.com/AcademySoftwareFoundation/OpenShadingLanguage

command = testshade("-g 5 1 test")
15 changes: 15 additions & 0 deletions testsuite/closure-conditional/test.osl
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// Copyright Contributors to the Open Shading Language project.
// SPDX-License-Identifier: BSD-3-Clause
// https://github.com/AcademySoftwareFoundation/OpenShadingLanguage

shader
test()
{
if (u > 0.5) {
Ci = u * emission();
} else {
Ci = color(1, 0, 0) * emission();
}

printf(" Ci = %s\n", Ci);
}

0 comments on commit 5dad507

Please sign in to comment.