beanz added inline comments.
================ Comment at: clang/lib/Headers/hlsl/hlsl_basic_types.h:30 #ifdef __HLSL_ENABLE_16_BIT -typedef int16_t int16_t2 __attribute__((ext_vector_type(2))); -typedef int16_t int16_t3 __attribute__((ext_vector_type(3))); -typedef int16_t int16_t4 __attribute__((ext_vector_type(4))); -typedef uint16_t uint16_t2 __attribute__((ext_vector_type(2))); -typedef uint16_t uint16_t3 __attribute__((ext_vector_type(3))); -typedef uint16_t uint16_t4 __attribute__((ext_vector_type(4))); +typedef vector<int16_t, 2> int16_t2; +typedef vector<int16_t, 3> int16_t3; ---------------- aaron.ballman wrote: > Can you explain these changes in light of: > > > The problem is that templates aren't supported before HLSL 2021, and type > > aliases still aren't supported in HLSL. > > This still looks like it's using template and type aliases. This is an extreme total hack of insanity, but it is how HLSL works. Basically, user-defined templates aren't supported, but built-in templates have been around for years. In DXC, the `vector` template is a class containing an ext_vector but there are horrible hacks in member expr handling to make the `.` operator return ext_vector swizzles... Using an alias allows me to get source compatibility here without having to do any of the member expr hacking. ================ Comment at: clang/lib/Sema/HLSLExternalSemaSource.cpp:41-44 + auto *UsingDecl = UsingDirectiveDecl::Create( + AST, AST.getTranslationUnitDecl(), SourceLocation(), SourceLocation(), + NestedNameSpecifierLoc(), SourceLocation(), HLSLNamespace, + AST.getTranslationUnitDecl()); ---------------- aaron.ballman wrote: > And users won't find this behavior surprising? I would be worried that the > user has their own globally declared type that this hidden using directive > will then cause to be ambiguous: https://godbolt.org/z/jMsW54vWe -- when > users hit this themselves, the location of the conflict is going to point > into nowhere-land, which is also rather unfortunate. > > Actually, the more I think about this, the less comfortable I am with it. > This also means you have to be *very* careful about conflicts with STL names, > and it means that any new declarations added to the `hlsl` namespace > automatically risk breaking code. > > Aside: any particular reason the namespace declaration is implicit but the > using directive declaration is not? As HLSL is implanted today in DXC the built-in types and functions are all globally defined (not in any namespace). My goal here was to start nesting them in an `hlsl` namespace, and to eventually do that for DXC as well in a new language version. HLSL just got support for `using namespace` like 3 months ago, so we can't do that in a header for compatibility, so my intent was to throw it in the directive here. My hope is the next version of HLSL will move the builtins into the `hlsl` namespace so I can make this only enabled for older language versions in the near future. In terms of conflicts with STL, I don't think the STL sources will compile for HLSL anytime soon. Maybe in a future version of HLSL, but we've got a long way to go to get there. For existing HLSL code, namespaces are very sparingly used, and while we may find conflicts in user code I think moving HLSL's builtins into a namespace will be worth the pain... but I might be wrong about how much pain that will be. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128012/new/ https://reviews.llvm.org/D128012 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
