Checkpoint to phase 2 of the great ustringhash conversion (#1612)

Reminder: phase 1 changed the RendererServices APIs to be based
completely on ustringhash for public method calls, not ustring.

This PR starts along the path of phase 2.

In oslconfig.h, I define a new type `ustringrep` which is the
in-memory/in-shader CPU-side representation of a ustring. Currently,
it's still equivalent to a ustring.

I also define a type called `ustring_pod`, which is the "plain old
data" representation inside a ustringrep (i.e., a const char* when
it's a ustring, and a uint64_t when it's a ustringhash). For reasons
not important, it's exceedingly hard to pass a true
ustring/ustringhash as parameters to or from LLVM-jitted code without
running afoul of LLVM's type checking, so it's just easier to pass the
underlying data type.

So the main work of phase 2 is to change everything related to shader
data from a ustring into a ustringrep, and things passed as const
char* into passing a ustring_pod. In some sense, it's just renaming
and type conversion plumbing.  When that's done, phase 3 is to throw
the switch and change the definition of ustringrep to ustringhash (and
of course fix everything that's broken).

This PR does not complete phase 2, but takes a big bite out of it.
It's an arbitrary bite, not much rhyme or reason to which specific
parts I did so far. Just working on it piece by piece and felt that
this was a place where much has been done, everything still works, and
it's a reviewable chunk without getting too much out of control.

Areas I worked on are related to range_check, raytype, getattribute,
bind_interpolated_param, uninit_check, range_check, naninf_check,
getmessage/setmessage, some texture setup, pointcloud.

Signed-off-by: Larry Gritz <[email protected]>

src/include/OSL/oslconfig.h.in

@@ -138,11 +138,22 @@ fmtformat(const Str& fmt, Args&&... args)
 #cmakedefine01 OSL_USTRINGREP_IS_HASH
 
 #if OSL_USTRINGREP_IS_HASH
+/// ustringrep is the class we use to represent strings in the shader.
 using ustringrep = ustringhash;
 #else
+/// ustringrep is the class we use to represent strings in the shader.
 using ustringrep = ustring;
 #endif
 
+#if OSL_USTRINGREP_IS_HASH
+/// ustring_pod is the type we use to pass string data in llvm function calls.
+using ustring_pod = size_t;
+#else
+/// ustring_pod is the type we use to pass string data in llvm function calls.
+using ustring_pod = const char*;
+#endif
+
+
 
 /// Convenience function to convert to a ustring.
 inline ustring
@@ -158,6 +169,28 @@ ustring_from(ustring u)
     return u;
 }
 
+/// Convenience function to convert to a ustring.
+inline ustring
+ustring_from(string_view s)
+{
+    return ustring(s);
+}
+
+
+/// Convenience function to convert to a ustringhash.
+inline ustringhash
+ustringhash_from(ustringhash u)
+{
+    return u;
+}
+
+/// Convenience function to convert to a ustringhash.
+inline ustringhash
+ustringhash_from(ustring u)
+{
+    return u.uhash();
+}
+
 
 /// Convenience function to convert to a ustringrep.
 inline ustringrep
@@ -181,6 +214,18 @@ ustringrep_from(ustring u)
 #endif
 }
 
+/// Convenience function to convert to a ustringrep.
+inline ustringrep
+ustringrep_from(string_view s)
+{
+    ustring u(s);
+#if OSL_USTRINGREP_IS_HASH
+    return u.hash();
+#else
+    return u;
+#endif
+}
+
 
 
 // N.B. SymArena is not really "configuration", but we cram it here for
@@ -279,7 +324,8 @@ OSL_NAMESPACE_EXIT
 
 namespace std {  // not necessary in C++17, then we can just say std::hash
 #ifndef OIIO_USTRING_HAS_STDHASH
-// Not a new enough OIIO to define std::hash<ustring>, so we'll do it
+// Not a new enough OIIO to define std::hash<ustring>, so we'll do it.
+// This can be removed once OIIO minimum is 2.4.5+ or >=2.5.0.1
 template<> struct hash<OSL::ustring> {
     std::size_t operator()(OSL::ustring u) const noexcept { return u.hash(); }
 };
@@ -293,7 +339,8 @@ template<> struct hash<OSL::ustringhash> {
 };
 #endif
 
-// Not necessary once minimum OIIO defines operator< for ustringhash
+// Not necessary once minimum OIIO defines operator< for ustringhash.
+// This can be removed once OIIO minimum is 2.4.5+ or >=2.5.0.1
 template<> struct less<OSL::ustringhash> {
     OSL_HOSTDEVICE constexpr bool operator()(OSL::ustringhash u,
                                              OSL::ustringhash v) const noexcept
@@ -302,3 +349,21 @@ template<> struct less<OSL::ustringhash> {
     }
 };
 }  // namespace std
+
+
+#ifndef OIIO_HAS_USTRINGHASH_FORMATTER
+// OIIO is too old to have fmt custom formatter for ustringhash.
+// This can be removed once OIIO minimum is 2.4.5+ or >=2.5.0.1
+FMT_BEGIN_NAMESPACE
+template<> struct formatter<OIIO::ustringhash> : formatter<fmt::string_view, char> {
+    template<typename FormatContext>
+    auto format(const OIIO::ustringhash& h, FormatContext& ctx)
+        -> decltype(ctx.out()) const
+    {
+        OIIO::ustring u(h);
+        return formatter<fmt::string_view, char>::format({ u.data(), u.size() },
+                                                         ctx);
+    }
+};
+FMT_END_NAMESPACE
+#endif

src/liboslexec/builtindecl.h

@@ -204,13 +204,13 @@ DECL(osl_dict_next, "iXi")
 DECL(osl_dict_value, "iXiXLX")
 DECL(osl_raytype_name, "iXs")
 #ifdef OSL_LLVM_NO_BITCODE
-DECL(osl_range_check, "iiiXXXiXiXX")
+DECL(osl_range_check, "iiisXsisiss")
 #endif
-DECL(osl_range_check_err, "iiiXXXiXiXX")
-DECL(osl_naninf_check, "xiXiXXiXiiX")
-DECL(osl_uninit_check, "xLXXXiXiXXiXiXii")
-DECL(osl_get_attribute, "iXiXXiiLX")
-DECL(osl_bind_interpolated_param, "iXXLiXiXiXi")
+DECL(osl_range_check_err, "iiisXsisiss")
+DECL(osl_naninf_check, "xiXiXsisiis")
+DECL(osl_uninit_check, "xLXXsisissisisii")
+DECL(osl_get_attribute, "iXissiiLX")
+DECL(osl_bind_interpolated_param, "iXsLiXiXiXi")
 DECL(osl_get_texture_options, "XX");
 DECL(osl_get_noise_options, "XX");
 DECL(osl_get_trace_options, "XX");
@@ -338,8 +338,8 @@ DECL(osl_substr_ssii, "ssii")
 DECL(osl_regex_impl, "iXsXisi")
 
 // Used by wide code generator, but are uniform calls
-DECL(osl_texture_decode_wrapmode, "iX");
-DECL(osl_texture_decode_interpmode, "iX");
+DECL(osl_texture_decode_wrapmode, "is");
+DECL(osl_texture_decode_interpmode, "is");
 
 DECL(osl_texture_set_firstchannel, "xXi")
 DECL(osl_texture_set_swrap, "xXs")

src/liboslexec/builtindecl_wide_xmacro.h

@@ -315,7 +315,7 @@ DECL(__OSL_MASKED_OP(pointcloud_get), "iXsXiXsLXi")
 DECL(__OSL_MASKED_OP(pointcloud_write), "iXsXiXXXi")
 
 DECL(__OSL_MASKED_OP(getmessage), "xXXssLXiisii")
-DECL(__OSL_MASKED_OP2(setmessage, s, WX), "xXXLXisii")
+DECL(__OSL_MASKED_OP2(setmessage, s, WX), "xXsLXisii")
 DECL(__OSL_MASKED_OP2(setmessage, Ws, WX), "xXXLXisii")
 
 DECL(__OSL_OP(blackbody_vf), "xXXf")
@@ -362,24 +362,24 @@ DECL(__OSL_OP(dict_value), "iXiXLX")
 DECL(__OSL_MASKED_OP(dict_value), "xXXXXLXi")
 
 
-DECL(__OSL_OP(raytype_name), "iXX")
+DECL(__OSL_OP(raytype_name), "iXs")
 DECL(__OSL_MASKED_OP(raytype_name), "xXXXi")
-DECL(__OSL_OP(naninf_check), "xiXiXXiXiiX")
-DECL(__OSL_MASKED_OP1(naninf_check_offset, i), "xiiXiXXiXiiX")
-DECL(__OSL_MASKED_OP1(naninf_check_offset, Wi), "xiiXiXXiXXiX")
-DECL(__OSL_OP(range_check), "iiiXXXiXiXX")
-DECL(__OSL_MASKED_OP(range_check), "xXiiXXXiXiXX")
-DECL(__OSL_OP2(uninit_check_values_offset, X, i), "xLXXXiXiXXiXiXii")
-DECL(__OSL_MASKED_OP2(uninit_check_values_offset, X, Wi), "xiLXXXiXiXXiXiXXi")
-DECL(__OSL_MASKED_OP2(uninit_check_values_offset, WX, i), "xiLXXXiXiXXiXiXii")
-DECL(__OSL_MASKED_OP2(uninit_check_values_offset, WX, Wi), "xiLXXXiXiXXiXiXXi")
-
-DECL(__OSL_OP1(get_attribute, s), "iXiXXiiXXi")
-DECL(__OSL_MASKED_OP1(get_attribute, Ws), "iXiXXiiXXi")
-DECL(__OSL_OP(get_attribute_uniform), "iXiXXiiXX")
+DECL(__OSL_OP(naninf_check), "xiXiXsisiis")
+DECL(__OSL_MASKED_OP1(naninf_check_offset, i), "xiiXiXsisiis")
+DECL(__OSL_MASKED_OP1(naninf_check_offset, Wi), "xiiXiXsisXis")
+DECL(__OSL_OP(range_check), "iiisXsisiss")
+DECL(__OSL_MASKED_OP(range_check), "xXiisXsisiss")
+DECL(__OSL_OP2(uninit_check_values_offset, X, i), "xLXXsisissisisii")
+DECL(__OSL_MASKED_OP2(uninit_check_values_offset, X, Wi), "xiLXXsisissisisXi")
+DECL(__OSL_MASKED_OP2(uninit_check_values_offset, WX, i), "xiLXXsisissisisii")
+DECL(__OSL_MASKED_OP2(uninit_check_values_offset, WX, Wi), "xiLXXsisissisisXi")
+
+DECL(__OSL_OP1(get_attribute, s), "iXissiiXXi")
+DECL(__OSL_MASKED_OP1(get_attribute, Ws), "iXisXiiXXi")
+DECL(__OSL_OP(get_attribute_uniform), "iXissiiXX")
 
 // TODO:  shouldn't bind_interpolated_param be MASKED?  change name to reflect
-DECL(__OSL_OP(bind_interpolated_param), "iXXLiXiXiXii")
+DECL(__OSL_OP(bind_interpolated_param), "iXsLiXiXiXii")
 
 //DECL (osl_get_texture_options, "XX") // unneeded
 DECL(__OSL_OP(get_noise_options), "XX")

src/liboslexec/context.cpp

@@ -625,8 +625,8 @@ ShadingContext::find_regex(ustring r)
 
 bool
 ShadingContext::osl_get_attribute(ShaderGlobals* sg, void* objdata,
-                                  int dest_derivs, ustring obj_name,
-                                  ustring attr_name, int array_lookup,
+                                  int dest_derivs, ustringhash obj_name,
+                                  ustringhash attr_name, int array_lookup,
                                   int index, TypeDesc attr_type,
                                   void* attr_dest)
 {

src/liboslexec/llvm_ops.cpp

@@ -27,9 +27,8 @@ examples), as you are just coding in C++, but there are some rules:
   float.  Aggregates (color/point/vector/normal/matrix), arrays of any
   types, or floats with derivatives are passed as a void* and to their
   memory location you need to cast appropriately.  Strings are passed as
-  char*, but they are always the characters of 'ustring' objects, so are
-  unique.  See the handy USTR, MAT, VEC, DFLOAT, DVEC macros for
-  handy/cheap casting of those void*'s to references to ustring&,
+  ustringrep.  See the handy USTR, MAT, VEC, DFLOAT, DVEC macros for
+  handy/cheap casting of those void*'s to references to ustringrep&,
   Matrix44&, Vec3&, Dual2<float>&, and Dual2<Vec3>, respectively.
 
 * You must provide all allowable polymorphic and derivative combinations!
@@ -101,13 +100,23 @@ void* __dso_handle = 0;  // necessary to avoid linkage issues in bitcode
 
 
 // Handy re-casting macros
-#define USTR(cstr) (*((ustring*)&cstr))
-#define MAT(m)     (*(Matrix44*)m)
-#define VEC(v)     (*(Vec3*)v)
-#define DFLOAT(x)  (*(Dual2<Float>*)x)
-#define DVEC(x)    (*(Dual2<Vec3>*)x)
-#define COL(x)     (*(Color3*)x)
-#define DCOL(x)    (*(Dual2<Color3>*)x)
+#define USTR(s)   (*((ustringrep*)&s))
+#define USTREP(s) (*((ustringrep*)&s))
+#define MAT(m)    (*(Matrix44*)m)
+#define VEC(v)    (*(Vec3*)v)
+#define DFLOAT(x) (*(Dual2<Float>*)x)
+#define DVEC(x)   (*(Dual2<Vec3>*)x)
+#define COL(x)    (*(Color3*)x)
+#define DCOL(x)   (*(Dual2<Color3>*)x)
+
+#if OSL_USTRINGREP_IS_HASH
+/// ustring_pod is the type we use to pass string data in llvm function calls.
+using ustring_pod = size_t;
+#else
+/// ustring_pod is the type we use to pass string data in llvm function calls.
+using ustring_pod = const char*;
+#endif
+
 
 #ifndef OSL_SHADEOP
 #    ifdef __CUDACC__
@@ -975,17 +984,17 @@ osl_raytype_bit(void* sg_, int bit)
 
 // extern declaration
 OSL_SHADEOP_NOINLINE int
-osl_range_check_err(int indexvalue, int length, const char* symname, void* sg,
-                    const void* sourcefile, int sourceline,
-                    const char* groupname, int layer, const char* layername,
-                    const char* shadername);
+osl_range_check_err(int indexvalue, int length, ustring_pod symname, void* sg,
+                    ustring_pod sourcefile, int sourceline,
+                    ustring_pod groupname, int layer, ustring_pod layername,
+                    ustring_pod shadername);
 
 
 
 OSL_SHADEOP int
-osl_range_check(int indexvalue, int length, const char* symname, void* sg,
-                const void* sourcefile, int sourceline, const char* groupname,
-                int layer, const char* layername, const char* shadername)
+osl_range_check(int indexvalue, int length, ustring_pod symname, void* sg,
+                ustring_pod sourcefile, int sourceline, ustring_pod groupname,
+                int layer, ustring_pod layername, ustring_pod shadername)
 {
     if (indexvalue < 0 || indexvalue >= length) {
         indexvalue = osl_range_check_err(indexvalue, length, symname, sg,

src/liboslexec/llvm_util.cpp

@@ -2455,6 +2455,11 @@ LLVM_Util::make_function(const std::string& name, bool fastcall,
     llvm::FunctionType* functype = type_function(rettype, params, varargs);
     auto maybe_func = module()->getOrInsertFunction(name, functype).getCallee();
     OSL_ASSERT(maybe_func && "getOrInsertFunction returned NULL");
+    // if (!llvm::isa<llvm::Function>(maybe_func)) {
+    //     print("make_function: getOrInsertFunction returned non-function for {}\n", name);
+    //     for (auto p : params)
+    //         print("   param type: {}\n", llvm_typename(p));
+    // }
     OSL_ASSERT_MSG(llvm::isa<llvm::Function>(maybe_func),
                    "Declaration for %s is wrong, LLVM had to make a cast",
                    name.c_str());
@@ -2850,15 +2855,21 @@ LLVM_Util::constant(ustring s)
     const size_t size_t_bits = sizeof(size_t) * 8;
     // Create a const size_t with the ustring character address, or hash,
     // depending on the representation we're using.
-    size_t p = (ustring_rep() == UstringRep::charptr) ? size_t(s.c_str())
-                                                      : s.hash();
-    auto str = (size_t_bits == 64) ? constant64(p) : constant(int(p));
-    // Then cast the int to a char*. Ideally, we would only do that if the rep
-    // were a charptr, but we disguise the hashes as char*'s also to avoid
-    // ugliness with function signatures differing between CPU and GPU.
-    // Maybe some day we'll use the hash representation on both sides?
-    return builder().CreateIntToPtr(str, m_llvm_type_ustring,
-                                    "ustring constant");
+    if (ustring_rep() == UstringRep::charptr) {
+        return constant_ptr((void*)s.c_str(), type_char_ptr());
+    } else {
+        size_t p = s.hash();
+        auto str = (size_t_bits == 64) ? constant64(p) : constant(int(p));
+#if OSL_USTRINGREP_IS_HASH
+        return str;
+#else
+        // Then cast the int to a char*. Ideally, we would only do that if the rep
+        // were a charptr, but we disguise the hashes as char*'s also to avoid
+        // ugliness with function signatures differing between CPU and GPU.
+        return builder().CreateIntToPtr(str, m_llvm_type_ustring,
+                                        "ustring constant");
+#endif
+    }
 }
 
 
@@ -3591,9 +3602,9 @@ LLVM_Util::call_function(llvm::Value* func, cspan<llvm::Value*> args)
     OSL_DASSERT(func);
 #if 0
     llvm::outs() << "llvm_call_function " << *func << "\n";
-    llvm::outs() << nargs << " args:\n";
-    for (int i = 0, nargs = args.size();  i < nargs;  ++i)
-        llvm::outs() << "\t" << *(args[i]) << "\n";
+    llvm::outs() << args.size() << " args:\n";
+    for (auto a : args)
+        llvm::outs() << "\t" << *a << "\n";
 #endif
     //llvm_gen_debug_printf (std::string("start ") + std::string(name));
 #if OSL_LLVM_VERSION >= 110