Skip to content

Commit

Permalink
fix: Handle mixed string casting that plagues optix codegen
Browse files Browse the repository at this point in the history
The string situation is still a bit tricky on the OptiX side.  There's
a full overhaul of that coming separately as we continue to shift to
an all-ustringhash world.

But in the mean time, we were getting some testsuite failures that
were because of a mismatch between times we think of a gpu-side
ustring as a char* and when it's a uint64. IN FACT, they are always
the hashes, but we backed ourselves into a corner where sometimes that
64 bit pattern is passed around as a pointer, ugh.

This small patch attempts to catch this case and just do the
appropriate cast to compensate. It's a band-aid, a more comprehensive
fix is coming, as I said.

This gets us down to just ONE filing optix test case, which I will
tackle separately.

Signed-off-by: Larry Gritz <[email protected]>
  • Loading branch information
lgritz committed Aug 23, 2023
1 parent 271f8bc commit 7f2ad40
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 0 deletions.
4 changes: 4 additions & 0 deletions src/include/OSL/llvm_util.h
Original file line number Diff line number Diff line change
Expand Up @@ -762,6 +762,10 @@ class OSLEXECPUBLIC LLVM_Util {
/// return the llvm::Value of the new pointer.
llvm::Value* int_to_ptr_cast(llvm::Value* val);

/// Cast the pointer specified by ptr to an int64, return the llvm::Value
/// of the new value.
llvm::Value* ptr_to_int64_cast(llvm::Value* ptr);

/// Cast the pointer variable specified by val to a pointer of type
/// void* return the llvm::Value of the new pointer.
llvm::Value* void_ptr(llvm::Value* val, const std::string& llname = {});
Expand Down
7 changes: 7 additions & 0 deletions src/liboslexec/backendllvm.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -665,6 +665,13 @@ BackendLLVM::llvm_store_value(llvm::Value* new_val, llvm::Value* dst_ptr,
dst_ptr = ll.GEP(dst_ptr, 0, component);

// Finally, store the value.
if (t == TypeString && dst_ptr->getType() == ll.type_int64_ptr()
&& new_val->getType() == ll.type_char_ptr()) {
// Special case: we are still ickily storing strings sometimes as a
// char* and sometimes as a uint64. Do a little sneaky conversion
// here.
new_val = ll.ptr_to_int64_cast(new_val);
}
ll.op_store(new_val, dst_ptr);
return true;
}
Expand Down
9 changes: 9 additions & 0 deletions src/liboslexec/llvm_util.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3429,6 +3429,14 @@ LLVM_Util::int_to_ptr_cast(llvm::Value* val)



llvm::Value*
LLVM_Util::ptr_to_int64_cast(llvm::Value* ptr)
{
return builder().CreatePtrToInt(ptr, type_int64());
}



llvm::Value*
LLVM_Util::void_ptr(llvm::Value* val, const std::string& llname)
{
Expand Down Expand Up @@ -5216,6 +5224,7 @@ LLVM_Util::op_store(llvm::Value* val, llvm::Value* ptr)
std::cerr << "op_store val->getType()=" << std::flush;
val->getType()->print(llvm::errs());
std::cerr << std::endl;
// OSL_ASSERT(0);
}
if (m_mask_stack.empty() || val->getType()->isVectorTy() == false
|| (!is_masking_required())) {
Expand Down

0 comments on commit 7f2ad40

Please sign in to comment.