Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Remove direct access to ShaderGlobals members from llvm_ops #1414

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Remove direct access to ShaderGlobals members from llvm_ops #1414

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions src/include/OSL/llvm_util.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -804,6 +804,9 @@ class OSLEXECPUBLIC LLVM_Util {
/// Dereference a pointer: return *ptr
llvm::Value *op_load (llvm::Value *ptr);

/// Dereference an int ptr (casting ptr type if necessary)
llvm::Value *op_load_int (llvm::Value *ptr);

llvm::Value *op_gather(llvm::Value *ptr, llvm::Value *index);

/// Store to a dereferenced pointer
Expand Down Expand Up @@ -858,6 +861,10 @@ class OSLEXECPUBLIC LLVM_Util {
llvm::Value *op_int_to_bool (llvm::Value *a);
llvm::Value *op_float_to_double (llvm::Value *a);
llvm::Value *op_int_to_longlong (llvm::Value *a);
// int_to_bool_to_int is essentially turning any nonzero -> 1
llvm::Value *op_int_to_bool_to_int (llvm::Value *a) {
return op_bool_to_int(op_int_to_bool(a));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does op_int_to_bool_to_int generate better code than opt_bool_to_int(op_ne(a, constant(0)))?
If so might be useful elsewhere.

}

llvm::Value *op_and (llvm::Value *a, llvm::Value *b);
llvm::Value *op_or (llvm::Value *a, llvm::Value *b);
Expand Down
1 change: 1 addition & 0 deletions src/include/OSL/strdecls.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ STRDECL("end", end)
STRDECL("useparam", useparam)
STRDECL("!!!uninitialized!!!", uninitialized_string)
STRDECL("unull", unull)
STRDECL("flipHandedness", flipHandedness)
STRDECL("raytype", raytype)
STRDECL("color", color)
STRDECL("point", point)
Expand Down
1 change: 0 additions & 1 deletion src/liboslexec/batched_analysis.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,6 @@ static ustring op_texture3d("texture3d");
// Shader global strings
static ustring object2common("object2common");
static ustring shader2common("shader2common");
static ustring flipHandedness("flipHandedness");
} // namespace Strings

namespace pvt {
Expand Down
1 change: 0 additions & 1 deletion src/liboslexec/batched_backendllvm.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ static ustring backfacing("backfacing");
static ustring surfacearea("surfacearea");
static ustring object2common("object2common");
static ustring shader2common("shader2common");
static ustring flipHandedness("flipHandedness");
} // namespace Strings

namespace pvt {
Expand Down
4 changes: 4 additions & 0 deletions src/liboslexec/batched_backendllvm.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -324,6 +324,10 @@ class BatchedBackendLLVM : public OSOProcessorBase {
/// Retrieve the named global ("P", "N", etc.).
/// is_uniform is output parameter
llvm::Value* llvm_global_symbol_ptr(ustring name, bool& is_uniform);
llvm::Value* llvm_global_symbol_ptr(ustring name) {
bool is_uniform;
return llvm_global_symbol_ptr(name, is_uniform);
}

/// Test whether val is nonzero, return the llvm::Value* that's the
/// result of a CreateICmpNE or CreateFCmpUNE (depending on the
Expand Down
20 changes: 8 additions & 12 deletions src/liboslexec/batched_llvm_gen.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4054,8 +4054,8 @@ LLVMGEN (llvm_gen_calculatenormal)

llvm::Value *args[] = {
rop.llvm_void_ptr (Result),
rop.sg_void_ptr(),
rop.llvm_load_arg (P, true /*derivs*/, false /*op_is_uniform*/),
rop.llvm_global_symbol_ptr(Strings::flipHandedness),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a matter of sanity checking, might consider pulling the global_symbol_ptr out and passing the optional is_uniform parameter and OSL_ASSERT(!is_uniform) as the function being called will interpret the pointer as varying.

nullptr};
int arg_count = 3;

Expand Down Expand Up @@ -4313,19 +4313,15 @@ LLVMGEN (llvm_gen_raytype)
llvm::Value * sg = rop.sg_void_ptr();
if (Name.is_constant()) {
// We can statically determine the bit pattern
ustring name = ((ustring *)Name.data())[0];
llvm::Value *args[] = {
sg,
rop.ll.constant (rop.shadingsys().raytype_bit (name))};

llvm::Value *ret = rop.ll.call_function (rop.build_name("raytype_bit"), args);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should be able to remove
OSL_BATCHOP int _OSL_OP(raytype_bit)(void* bsg, int bit)
from wide_shadingsys.cpp, and
DECL(__OSL_OP(raytype_bit), "iXi")
from builtindecl_wide_xmacro.h


if(!result_is_uniform)
{
llvm::Value* bit;
bit = rop.ll.constant(rop.shadingsys().raytype_bit(Name.get_string()));
llvm::Value* raytype = rop.ll.op_load_int(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wondering if less is more, meaning was op_load_int needed, in that LLVM_Util::ptr_to_cast or LLVM_Util::ptr_cast and LLVM_Util::op_load which could be used to provide the same functionality.

rop.llvm_global_symbol_ptr(Strings::raytype));
llvm::Value* bitanded = rop.ll.op_and(raytype, bit);
llvm::Value* ret = rop.ll.op_int_to_bool_to_int(bitanded);
if (!result_is_uniform)
ret = rop.ll.widen_value(ret);
}
rop.llvm_store_value (ret, Result);

} else {
FuncSpec func_spec("raytype_name");
// No way to know which name is being asked for
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the non-const case suggest following the same pattern you did for llvm_gen, go into wide_shading.cpp and rename the uniform and varying versions of raytype_name to raytype_bit.

For the uniform verson of raytype_name just return the bit by removing the bitwise AND return (bsg->uniform.raytype & bit), and in the batched_llvm_gen add a bitwise AND to the raytype from batchedShaderGlobals (very similar to llvm_gen version).

For the varying version, leave the foreach_unique loop, but again drop the bitwise AND int ray_is_named_type = ((bsg->uniform.raytype & bit) != 0) and just pass the bit variable into assign_all. But back in the batched_llvm_gen you will now need to allocate a varying temporary int to pass to varying version of raytype_bit. IE:

BatchedBackendLLVM::TempScope temp_scope(rop);

// We need wide place to hold the results of raytype_bit 
llvm::Value * loc_raytype_bit = rop.getOrAllocateTemp (TypeSpec(TypeDesc::INT), false /*derivs*/, false /*is_uniform*/, false /*forceBool*/, std::string("raytype_bit"));
            
llvm::Value *args[] = {
                sg,
                rop.llvm_void_ptr (loc_raytype_bit ),
                rop.llvm_void_ptr (Name),
                rop.ll.mask_as_int(rop.ll.current_mask())};
rop.ll.call_function (rop.build_name(func_spec), args);

llvm::Value* raytype= rop.ll.op_load(loc_raytype_bit );
llvm::Value* wide_bit = rop.ll.widen_value(bit);
llvm::Value* bitanded = rop.ll.op_and(raytype, wide_bit );
llvm::Value* ret = rop.ll.op_int_to_bool_to_int(bitanded);
OSL_DASSERT(!result_is_uniform);
rop.llvm_store_value (ret, Result);

To simplify, you can can probably combine some of the code between the uniform and non-uniform branches

Expand Down
6 changes: 3 additions & 3 deletions src/liboslexec/builtindecl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ DECL (osl_dict_find_iis, "iXiX")
DECL (osl_dict_find_iss, "iXXX")
DECL (osl_dict_next, "iXi")
DECL (osl_dict_value, "iXiXLX")
DECL (osl_raytype_name, "iXX")
DECL (osl_raytype_bit, "iXX")
#ifdef OSL_LLVM_NO_BITCODE
DECL (osl_range_check, "iiiXXXiXiXX")
#endif
Expand Down Expand Up @@ -376,11 +376,11 @@ DECL (osl_trace_set_traceset, "xXs")
DECL (osl_trace, "iXXXXXXXX")

#ifdef OSL_LLVM_NO_BITCODE
DECL (osl_calculatenormal, "xXXX")
DECL (osl_calculatenormal, "xXXi")
DECL (osl_area, "fX")
DECL (osl_filterwidth_fdf, "fX")
DECL (osl_filterwidth_vdv, "xXX")
DECL (osl_raytype_bit, "iXi")
DECL (osl_raytype_bit_from_name, "iXi")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see osl_raytype_bit_from_name referenced anywhere?
Although I do support using that name instead of just osl_raytype_bit.

#endif


Expand Down
28 changes: 15 additions & 13 deletions src/liboslexec/llvm_gen.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3165,11 +3165,11 @@ LLVMGEN (llvm_gen_calculatenormal)
rop.llvm_assign_zero (Result);
return true;
}

llvm::Value * args[] = {
rop.llvm_void_ptr (Result),
rop.sg_void_ptr(),
rop.llvm_void_ptr (P),
rop.llvm_void_ptr(Result),
rop.llvm_void_ptr(P),
rop.ll.op_load_int(rop.llvm_global_symbol_ptr(Strings::flipHandedness))
};
rop.ll.call_function ("osl_calculatenormal", args);
if (Result.has_derivs())
Expand Down Expand Up @@ -3766,20 +3766,22 @@ LLVMGEN (llvm_gen_raytype)
OSL_DASSERT(op.nargs() == 2);
Symbol& Result = *rop.opargsym (op, 0);
Symbol& Name = *rop.opargsym (op, 1);
llvm::Value *args[2] = { rop.sg_void_ptr(), NULL };
const char *func = NULL;

llvm::Value* bit = nullptr;
if (Name.is_constant()) {
// We can statically determine the bit pattern
ustring name = Name.get_string();
args[1] = rop.ll.constant (rop.shadingsys().raytype_bit (name));
func = "osl_raytype_bit";
bit = rop.ll.constant(rop.shadingsys().raytype_bit(Name.get_string()));
} else {
// No way to know which name is being asked for
args[1] = rop.llvm_get_pointer (Name);
func = "osl_raytype_name";
bit = rop.ll.call_function("osl_raytype_bit",
rop.sg_void_ptr(),
rop.llvm_get_pointer(Name));
}
llvm::Value *ret = rop.ll.call_function (func, args);
rop.llvm_store_value (ret, Result);

llvm::Value* raytype = rop.ll.op_load_int(
rop.llvm_global_symbol_ptr(Strings::raytype));
llvm::Value* bitanded = rop.ll.op_and(raytype, bit);
rop.llvm_store_value (rop.ll.op_int_to_bool_to_int(bitanded), Result);
return true;
}

Expand Down
20 changes: 2 additions & 18 deletions src/liboslexec/llvm_ops.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,11 +59,6 @@ examples), as you are just coding in C++, but there are some rules:
may write templates or helper functions (which do NOT need to use
OSL_SHADEOP, since they don't need to be runtime-discoverable by LLVM.

* If you need to access non-passed globals (P, N, etc.) or make renderer
callbacks, just make the first argument to the function a void* that
you cast to a ShaderGlobals* and access the globals, shading
context (sg->context), opaque renderer state (sg->renderstate), etc.

*/


Expand All @@ -78,7 +73,6 @@ typedef long double max_align_t;
#include <cstddef>

#include <OSL/oslconfig.h>
#include <OSL/shaderglobals.h>
#include <OSL/dual.h>
#include <OSL/dual_vec.h>
using namespace OSL;
Expand Down Expand Up @@ -768,10 +762,9 @@ OSL_HOSTDEVICE inline Vec3 calculatenormal(void *P_, bool flipHandedness)
return tmpP.dx().cross( tmpP.dy());
}

OSL_SHADEOP void osl_calculatenormal(void *out, void *sg_, void *P_)
OSL_SHADEOP void osl_calculatenormal(void *out, void *P_, int flipHandedness)
{
ShaderGlobals *sg = (ShaderGlobals *)sg_;
Vec3 N = calculatenormal(P_, sg->flipHandedness);
Vec3 N = calculatenormal(P_, flipHandedness);
// Don't normalize N
VEC(out) = N;
}
Expand Down Expand Up @@ -806,15 +799,6 @@ OSL_SHADEOP void osl_filterwidth_vdv(void *out, void *x_)



// Asked if the raytype includes a bit pattern.
OSL_SHADEOP int osl_raytype_bit (void *sg_, int bit)
{
ShaderGlobals *sg = (ShaderGlobals *)sg_;
return (sg->raytype & bit) != 0;
}



// extern declaration
OSL_SHADEOP_NOINLINE int osl_range_check_err (int indexvalue, int length,
const char *symname, void *sg,
Expand Down
10 changes: 9 additions & 1 deletion src/liboslexec/llvm_util.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3264,7 +3264,7 @@ LLVM_Util::ptr_to_cast (llvm::Value* val, llvm::Type *type)
llvm::Value *
LLVM_Util::ptr_cast (llvm::Value* val, llvm::Type *type)
{
return builder().CreatePointerCast(val,type);
return val->getType() == type ? val : builder().CreatePointerCast(val,type);
}


Expand Down Expand Up @@ -3600,6 +3600,14 @@ LLVM_Util::op_load (llvm::Value *ptr)
}


llvm::Value*
LLVM_Util::op_load_int(llvm::Value* ptr)
{
return op_load(ptr_cast(ptr, type_int_ptr()));
}




llvm::Value *
LLVM_Util::op_linearize_16x_indices(llvm::Value *wide_index)
Expand Down
1 change: 1 addition & 0 deletions src/liboslexec/oslexec_pvt.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -631,6 +631,7 @@ class ShadingSystemImpl
/// Set the current color space.
bool set_colorspace (ustring colorspace);

/// Return the bit that corresponds to the named ray type.
int raytype_bit (ustring name);

void optimize_all_groups (int nthreads=0, int mythread=0, int totalthreads=1, bool do_jit=true);
Expand Down
8 changes: 5 additions & 3 deletions src/liboslexec/shadingsys.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4174,15 +4174,17 @@ osl_range_check_err (int indexvalue, int length, const char *symname,



// Return the raytype bit corresponding to a raytype name.
// Asked if the raytype is a name we can't know until mid-shader.
OSL_SHADEOP int osl_raytype_name (void *sg_, void *name)
// FIXME: We should pass the context, not the sg.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why a FIXME, any technical barrier to passing context? For that matter, I've been pondering a ReadOnlyContext or DeviceContext that might be a stripped down version of the full context to answer just these types of questions.

OSL_SHADEOP int osl_raytype_bit (void *sg_, void *name)
{
ShaderGlobals *sg = (ShaderGlobals *)sg_;
int bit = sg->context->shadingsys().raytype_bit (USTR(name));
return (sg->raytype & bit) != 0;
return sg->context->shadingsys().raytype_bit(USTR(name));
}



OSL_SHADEOP int osl_get_attribute(void *sg_,
int dest_derivs,
void *obj_name_,
Expand Down
13 changes: 5 additions & 8 deletions src/liboslexec/wide/wide_opalgebraic.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -984,13 +984,12 @@ __OSL_MASKED_OP2(filterwidth,Wv,Wdv)

OSL_BATCHOP void
__OSL_OP(calculatenormal)
(void *out, void *bsg_, void *P_)
(void *out, void *P_, void* flip_)
{
auto *bsg = reinterpret_cast<BatchedShaderGlobals *>(bsg_);
OSL_FORCEINLINE_BLOCK
{
Wide<const Dual2<Vec3>> wP(P_);
Wide<const int> wFlipHandedness(bsg->varying.flipHandedness);
Wide<const int> wFlipHandedness(flip_);
Wide<Vec3> wr(out);

OSL_OMP_PRAGMA(omp simd simdlen(__OSL_WIDTH))
Expand All @@ -1004,21 +1003,19 @@ __OSL_OP(calculatenormal)

OSL_BATCHOP void
__OSL_MASKED_OP(calculatenormal)
(void *out, void *bsg_, void *P_, unsigned int mask_value)
(void *out, void *P_, void* flip_, unsigned int mask_value)
{
auto *bsg = reinterpret_cast<BatchedShaderGlobals *>(bsg_);
OSL_FORCEINLINE_BLOCK
{
Wide<const Dual2<Vec3>> wP(P_);
Wide<const int> wFlipHandedness(bsg->varying.flipHandedness);
Wide<const int> wFlipHandedness(flip_);
Masked<Vec3> wr(out, Mask(mask_value));

OSL_OMP_PRAGMA(omp simd simdlen(__OSL_WIDTH))
for(int lane=0; lane < __OSL_WIDTH; ++lane) {
Dual2<Vec3> P = wP[lane];
int flip_handedness = wFlipHandedness[lane];
//std::cout << "P=" << P.val() << "," << P.dx() << "," << P.dy() << std::endl;

//std::cout << "P=" << P << std::endl;
if (wr.mask()[lane]) {
Vec3 N = calculatenormal(P, flip_handedness);
wr[ActiveLane(lane)] = N;
Expand Down