rnk accepted this revision. rnk added a comment. Herald added a subscriber: StephenFan.
lgtm ================ Comment at: clang/lib/Frontend/InitPreprocessor.cpp:401-403 + uint32_t StageInteger = StageInteger = + (uint32_t)TI.getTriple().getEnvironment() - + (uint32_t)llvm::Triple::Pixel; ---------------- This here suggests that the order of the HLSL environments in the triple really, really matter, since we expose them to users. That's really surprising: it means small changes in LLVM affect this pre-processor macro in surprising ways. I see the test for this, but what do you think about adding a static_assert to Triple.cpp to move the check into LLVM? Something like: static_assert(Triple::Compute - Triple::Pixel == 1, "incorrect HLSL env order"); static_assert(Triple::Amplification - Triple::Pixel == 2, "incorrect HLSL env order"); Maybe this is addressed elsewhere, I haven't looked at all patches holistically. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122087/new/ https://reviews.llvm.org/D122087 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
