[PATCH] D159103: [Driver][HLSL] Improve diagnostics for invalid shader model and stage

[Justin Bogner via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGe504194d51e1: [Driver][HLSL] Improve diagnostics for invalid 
shader model and stage (authored by bogner).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D159103/new/

https://reviews.llvm.org/D159103

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/test/Driver/hlsl-lang-targets-spirv.hlsl
  clang/test/Driver/hlsl-lang-targets.hlsl
  llvm/include/llvm/TargetParser/Triple.h

Index: llvm/include/llvm/TargetParser/Triple.h
===
--- llvm/include/llvm/TargetParser/Triple.h
+++ llvm/include/llvm/TargetParser/Triple.h
@@ -271,7 +271,9 @@
 Callable,
 Mesh,
 Amplification,
+
 OpenHOS,
+
 LastEnvironmentType = OpenHOS
   };
   enum ObjectFormatType {
@@ -756,6 +758,22 @@
 return getArch() == Triple::dxil;
   }
 
+  bool isShaderModelOS() const {
+return getOS() == Triple::ShaderModel;
+  }
+
+  bool isShaderStageEnvironment() const {
+EnvironmentType Env = getEnvironment();
+return Env == Triple::Pixel || Env == Triple::Vertex ||
+   Env == Triple::Geometry || Env == Triple::Hull ||
+   Env == Triple::Domain || Env == Triple::Compute ||
+   Env == Triple::Library || Env == Triple::RayGeneration ||
+   Env == Triple::Intersection || Env == Triple::AnyHit ||
+   Env == Triple::ClosestHit || Env == Triple::Miss ||
+   Env == Triple::Callable || Env == Triple::Mesh ||
+   Env == Triple::Amplification;
+  }
+
   /// Tests whether the target is SPIR (32- or 64-bit).
   bool isSPIR() const {
 return getArch() == Triple::spir || getArch() == Triple::spir64;
@@ -767,6 +785,11 @@
getArch() == Triple::spirv;
   }
 
+  /// Tests whether the target is SPIR-V Logical
+  bool isSPIRVLogical() const {
+return getArch() == Triple::spirv;
+  }
+
   /// Tests whether the target is NVPTX (32- or 64-bit).
   bool isNVPTX() const {
 return getArch() == Triple::nvptx || getArch() == Triple::nvptx64;
Index: clang/test/Driver/hlsl-lang-targets.hlsl
===
--- clang/test/Driver/hlsl-lang-targets.hlsl
+++ clang/test/Driver/hlsl-lang-targets.hlsl
@@ -1,14 +1,53 @@
-// RUN: not %clang -target x86_64-unknown-unknown %s 2>&1 | FileCheck %s --check-prefix=X86
-// RUN: not %clang -target dxil-unknown-unknown %s 2>&1 | FileCheck %s --check-prefix=DXIL
-// RUN: not %clang -target x86_64-unknown-shadermodel %s 2>&1 | FileCheck %s --check-prefix=SM
-// RUN: not %clang -target spirv64-unknown-unknown %s 2>&1 | FileCheck %s --check-prefix=SPIRV
+// REQUIRES: dxil-registered-target
 
+// Supported targets
+//
+// RUN: %clang -target dxil--shadermodel6.2-pixel %s -S -o /dev/null 2>&1 | FileCheck --check-prefix=CHECK-VALID %s
+// RUN: %clang -target dxil-unknown-shadermodel6.2-pixel %s -S -o /dev/null 2>&1 | FileCheck --check-prefix=CHECK-VALID %s
+// RUN: %clang -target dxil--shadermodel6.2-library %s -S -o /dev/null 2>&1 | FileCheck --check-prefix=CHECK-VALID %s
+// RUN: %clang -target dxil-unknown-shadermodel6.2-library %s -S -o /dev/null 2>&1 | FileCheck --check-prefix=CHECK-VALID %s
 
-// A completely unsupported target...
-// X86: error: HLSL code generation is unsupported for target 'x86_64-unknown-unknown'
+// Empty shader model
+//
+// RUN: not %clang -target dxil %s -S -o /dev/null 2>&1 | FileCheck --check-prefix=CHECK-NO-OS %s
 
-// Poorly specified targets
-// DXIL: error: HLSL code generation is unsupported for target 'dxil-unknown-unknown'
-// SM: error: HLSL code generation is unsupported for target 'x86_64-unknown-shadermodel'
+// Invalid shader models
+//
+// RUN: not %clang -target dxil--linux %s -S -o /dev/null 2>&1 | FileCheck --check-prefix=CHECK-BAD-OS %s
+// RUN: not %clang -target dxil--win32 %s -S -o /dev/null 2>&1 | FileCheck --check-prefix=CHECK-BAD-OS %s
+// RUN: not %clang -target dxil--unknown %s -S -o /dev/null 2>&1 | FileCheck --check-prefix=CHECK-BAD-OS %s
+// RUN: not %clang -target dxil--invalidos %s -S -o /dev/null 2>&1 | FileCheck --check-prefix=CHECK-BAD-OS %s
 
-// FIXME// SPIRV: error: HLSL code generation is unsupported for target 'spirv64-unknown-unknown'
+// Bad shader model versions. Currently we just check for any version at all.
+//
+// RUN: not %clang -target dxil--shadermodel %s -S -o /dev/null 2>&1 | FileCheck --check-prefix=CHECK-BAD-OS %s
+// RUN: not %clang -target dxil--shadermodel0.0 %s -S -o /dev/null 2>&1 | FileCheck --check-prefix=CHECK-BAD-OS %s
+
+// Empty shader stage
+//
+// RUN: not %clang -target dxil-shadermodel6.2 %s -S -o /dev/null 2>&1 | FileCheck --check-prefix=CHECK-NO-ENV %s
+// RUN: not %clang -target dxil--shadermodel6.2 %s -S -o /dev/null 2>&1 | FileCheck 

[PATCH] D159103: [Driver][HLSL] Improve diagnostics for invalid shader model and stage

[Justin Bogner via Phabricator via cfe-commits]

bogner updated this revision to Diff 556589.
bogner added a comment.
Herald added a subscriber: pmatos.

- Check isSPIRVLogical rather than isSPIRV
- Improve diagnostics given that HLSL can generate both DXIL and SPIR-V
- Add tests for SPIR-V triples
- Fix missing `REQUIRES:` in dxil triple tests


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D159103/new/

https://reviews.llvm.org/D159103

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/test/Driver/hlsl-lang-targets-spirv.hlsl
  clang/test/Driver/hlsl-lang-targets.hlsl
  llvm/include/llvm/TargetParser/Triple.h

Index: llvm/include/llvm/TargetParser/Triple.h
===
--- llvm/include/llvm/TargetParser/Triple.h
+++ llvm/include/llvm/TargetParser/Triple.h
@@ -271,7 +271,9 @@
 Callable,
 Mesh,
 Amplification,
+
 OpenHOS,
+
 LastEnvironmentType = OpenHOS
   };
   enum ObjectFormatType {
@@ -756,6 +758,22 @@
 return getArch() == Triple::dxil;
   }
 
+  bool isShaderModelOS() const {
+return getOS() == Triple::ShaderModel;
+  }
+
+  bool isShaderStageEnvironment() const {
+EnvironmentType Env = getEnvironment();
+return Env == Triple::Pixel || Env == Triple::Vertex ||
+   Env == Triple::Geometry || Env == Triple::Hull ||
+   Env == Triple::Domain || Env == Triple::Compute ||
+   Env == Triple::Library || Env == Triple::RayGeneration ||
+   Env == Triple::Intersection || Env == Triple::AnyHit ||
+   Env == Triple::ClosestHit || Env == Triple::Miss ||
+   Env == Triple::Callable || Env == Triple::Mesh ||
+   Env == Triple::Amplification;
+  }
+
   /// Tests whether the target is SPIR (32- or 64-bit).
   bool isSPIR() const {
 return getArch() == Triple::spir || getArch() == Triple::spir64;
@@ -767,6 +785,11 @@
getArch() == Triple::spirv;
   }
 
+  /// Tests whether the target is SPIR-V Logical
+  bool isSPIRVLogical() const {
+return getArch() == Triple::spirv;
+  }
+
   /// Tests whether the target is NVPTX (32- or 64-bit).
   bool isNVPTX() const {
 return getArch() == Triple::nvptx || getArch() == Triple::nvptx64;
Index: clang/test/Driver/hlsl-lang-targets.hlsl
===
--- clang/test/Driver/hlsl-lang-targets.hlsl
+++ clang/test/Driver/hlsl-lang-targets.hlsl
@@ -1,14 +1,53 @@
-// RUN: not %clang -target x86_64-unknown-unknown %s 2>&1 | FileCheck %s --check-prefix=X86
-// RUN: not %clang -target dxil-unknown-unknown %s 2>&1 | FileCheck %s --check-prefix=DXIL
-// RUN: not %clang -target x86_64-unknown-shadermodel %s 2>&1 | FileCheck %s --check-prefix=SM
-// RUN: not %clang -target spirv64-unknown-unknown %s 2>&1 | FileCheck %s --check-prefix=SPIRV
+// REQUIRES: dxil-registered-target
 
+// Supported targets
+//
+// RUN: %clang -target dxil--shadermodel6.2-pixel %s -S -o /dev/null 2>&1 | FileCheck --check-prefix=CHECK-VALID %s
+// RUN: %clang -target dxil-unknown-shadermodel6.2-pixel %s -S -o /dev/null 2>&1 | FileCheck --check-prefix=CHECK-VALID %s
+// RUN: %clang -target dxil--shadermodel6.2-library %s -S -o /dev/null 2>&1 | FileCheck --check-prefix=CHECK-VALID %s
+// RUN: %clang -target dxil-unknown-shadermodel6.2-library %s -S -o /dev/null 2>&1 | FileCheck --check-prefix=CHECK-VALID %s
 
-// A completely unsupported target...
-// X86: error: HLSL code generation is unsupported for target 'x86_64-unknown-unknown'
+// Empty shader model
+//
+// RUN: not %clang -target dxil %s -S -o /dev/null 2>&1 | FileCheck --check-prefix=CHECK-NO-OS %s
 
-// Poorly specified targets
-// DXIL: error: HLSL code generation is unsupported for target 'dxil-unknown-unknown'
-// SM: error: HLSL code generation is unsupported for target 'x86_64-unknown-shadermodel'
+// Invalid shader models
+//
+// RUN: not %clang -target dxil--linux %s -S -o /dev/null 2>&1 | FileCheck --check-prefix=CHECK-BAD-OS %s
+// RUN: not %clang -target dxil--win32 %s -S -o /dev/null 2>&1 | FileCheck --check-prefix=CHECK-BAD-OS %s
+// RUN: not %clang -target dxil--unknown %s -S -o /dev/null 2>&1 | FileCheck --check-prefix=CHECK-BAD-OS %s
+// RUN: not %clang -target dxil--invalidos %s -S -o /dev/null 2>&1 | FileCheck --check-prefix=CHECK-BAD-OS %s
 
-// FIXME// SPIRV: error: HLSL code generation is unsupported for target 'spirv64-unknown-unknown'
+// Bad shader model versions. Currently we just check for any version at all.
+//
+// RUN: not %clang -target dxil--shadermodel %s -S -o /dev/null 2>&1 | FileCheck --check-prefix=CHECK-BAD-OS %s
+// RUN: not %clang -target dxil--shadermodel0.0 %s -S -o /dev/null 2>&1 | FileCheck --check-prefix=CHECK-BAD-OS %s
+
+// Empty shader stage
+//
+// RUN: not %clang -target dxil-shadermodel6.2 %s -S -o /dev/null 2>&1 | FileCheck --check-prefix=CHECK-NO-ENV %s
+// RUN: not %clang -target dxil--shadermodel6.2 %s 

[PATCH] D159103: [Driver][HLSL] Improve diagnostics for invalid shader model and stage

[Justin Bogner via Phabricator via cfe-commits]

bogner added a subscriber: Keenuts.
bogner added inline comments.



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:4146
   if (Opts.HLSL) {
-bool SupportedTarget = (T.getArch() == llvm::Triple::dxil ||
-T.getArch() == llvm::Triple::spirv) &&
-   T.getOS() == llvm::Triple::ShaderModel;
-if (!SupportedTarget)
+if (T.isDXIL() || T.isSPIRV) {
+  enum { ShaderModel, ShaderStage };

@Keenuts Does this look right, or do we want something like `T.isSPIRVLogical` 
here?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D159103/new/

https://reviews.llvm.org/D159103

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D159103: [Driver][HLSL] Improve diagnostics for invalid shader model and stage

[Justin Bogner via Phabricator via cfe-commits]

bogner updated this revision to Diff 556512.
bogner added a comment.

Rebased to fix conflicts


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D159103/new/

https://reviews.llvm.org/D159103

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/test/Driver/hlsl-lang-targets.hlsl
  llvm/include/llvm/TargetParser/Triple.h

Index: llvm/include/llvm/TargetParser/Triple.h
===
--- llvm/include/llvm/TargetParser/Triple.h
+++ llvm/include/llvm/TargetParser/Triple.h
@@ -271,7 +271,9 @@
 Callable,
 Mesh,
 Amplification,
+
 OpenHOS,
+
 LastEnvironmentType = OpenHOS
   };
   enum ObjectFormatType {
@@ -756,6 +758,22 @@
 return getArch() == Triple::dxil;
   }
 
+  bool isShaderModelOS() const {
+return getOS() == Triple::ShaderModel;
+  }
+
+  bool isShaderStageEnvironment() const {
+EnvironmentType Env = getEnvironment();
+return Env == Triple::Pixel || Env == Triple::Vertex ||
+   Env == Triple::Geometry || Env == Triple::Hull ||
+   Env == Triple::Domain || Env == Triple::Compute ||
+   Env == Triple::Library || Env == Triple::RayGeneration ||
+   Env == Triple::Intersection || Env == Triple::AnyHit ||
+   Env == Triple::ClosestHit || Env == Triple::Miss ||
+   Env == Triple::Callable || Env == Triple::Mesh ||
+   Env == Triple::Amplification;
+  }
+
   /// Tests whether the target is SPIR (32- or 64-bit).
   bool isSPIR() const {
 return getArch() == Triple::spir || getArch() == Triple::spir64;
Index: clang/test/Driver/hlsl-lang-targets.hlsl
===
--- clang/test/Driver/hlsl-lang-targets.hlsl
+++ clang/test/Driver/hlsl-lang-targets.hlsl
@@ -1,14 +1,52 @@
-// RUN: not %clang -target x86_64-unknown-unknown %s 2>&1 | FileCheck %s --check-prefix=X86
-// RUN: not %clang -target dxil-unknown-unknown %s 2>&1 | FileCheck %s --check-prefix=DXIL
-// RUN: not %clang -target x86_64-unknown-shadermodel %s 2>&1 | FileCheck %s --check-prefix=SM
-// RUN: not %clang -target spirv64-unknown-unknown %s 2>&1 | FileCheck %s --check-prefix=SPIRV
+// Supported targets
+//
+// RUN: %clang -target dxil--shadermodel6.2-pixel %s -S -o /dev/null 2>&1 | FileCheck --check-prefix=CHECK-VALID %s
+// RUN: %clang -target dxil-unknown-shadermodel6.2-pixel %s -S -o /dev/null 2>&1 | FileCheck --check-prefix=CHECK-VALID %s
+// RUN: %clang -target dxil--shadermodel6.2-library %s -S -o /dev/null 2>&1 | FileCheck --check-prefix=CHECK-VALID %s
+// RUN: %clang -target dxil-unknown-shadermodel6.2-library %s -S -o /dev/null 2>&1 | FileCheck --check-prefix=CHECK-VALID %s
 
+// Empty shader model
+//
+// RUN: not %clang -target dxil %s -S -o /dev/null 2>&1 | FileCheck --check-prefix=CHECK-NO-OS %s
 
-// A completely unsupported target...
-// X86: error: HLSL code generation is unsupported for target 'x86_64-unknown-unknown'
+// Invalid shader models
+//
+// RUN: not %clang -target dxil--linux %s -S -o /dev/null 2>&1 | FileCheck --check-prefix=CHECK-BAD-OS %s
+// RUN: not %clang -target dxil--win32 %s -S -o /dev/null 2>&1 | FileCheck --check-prefix=CHECK-BAD-OS %s
+// RUN: not %clang -target dxil--unknown %s -S -o /dev/null 2>&1 | FileCheck --check-prefix=CHECK-BAD-OS %s
+// RUN: not %clang -target dxil--invalidos %s -S -o /dev/null 2>&1 | FileCheck --check-prefix=CHECK-BAD-OS %s
 
-// Poorly specified targets
-// DXIL: error: HLSL code generation is unsupported for target 'dxil-unknown-unknown'
-// SM: error: HLSL code generation is unsupported for target 'x86_64-unknown-shadermodel'
+// Bad shader model versions. Currently we just check for any version at all.
+//
+// RUN: not %clang -target dxil--shadermodel %s -S -o /dev/null 2>&1 | FileCheck --check-prefix=CHECK-BAD-OS %s
+// RUN: not %clang -target dxil--shadermodel0.0 %s -S -o /dev/null 2>&1 | FileCheck --check-prefix=CHECK-BAD-OS %s
 
-// FIXME// SPIRV: error: HLSL code generation is unsupported for target 'spirv64-unknown-unknown'
+// Empty shader stage
+//
+// RUN: not %clang -target dxil-shadermodel6.2 %s -S -o /dev/null 2>&1 | FileCheck --check-prefix=CHECK-NO-ENV %s
+// RUN: not %clang -target dxil--shadermodel6.2 %s -S -o /dev/null 2>&1 | FileCheck --check-prefix=CHECK-NO-ENV %s
+// RUN: not %clang -target dxil--shadermodel6.2 %s -S -o /dev/null 2>&1 | FileCheck --check-prefix=CHECK-NO-ENV %s
+
+// Invalid shader stages
+//
+// RUN: not %clang -target dxil--shadermodel6.2-unknown %s -S -o /dev/null 2>&1 | FileCheck --check-prefix=CHECK-BAD-ENV %s
+// RUN: not %clang -target dxil--shadermodel6.2-invalidenvironment %s -S -o /dev/null 2>&1 | FileCheck --check-prefix=CHECK-BAD-ENV %s
+// RUN: not %clang -target dxil--shadermodel6.2-eabi %s -S -o /dev/null 2>&1 | FileCheck --check-prefix=CHECK-BAD-ENV %s
+// RUN: not %clang -target 

[PATCH] D159103: [Driver][HLSL] Improve diagnostics for invalid shader model and stage

[Justin Bogner via Phabricator via cfe-commits]

bogner added a comment.

Ping. Please take another look when you get a chance, thanks!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D159103/new/

https://reviews.llvm.org/D159103

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D159103: [Driver][HLSL] Improve diagnostics for invalid shader model and stage

[Justin Bogner via Phabricator via cfe-commits]

bogner updated this revision to Diff 555451.
bogner added a comment.

- Consolidate error messages
- Error messages should start with a lower case letter


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D159103/new/

https://reviews.llvm.org/D159103

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/test/Driver/hlsl-lang-targets.hlsl
  llvm/include/llvm/TargetParser/Triple.h

Index: llvm/include/llvm/TargetParser/Triple.h
===
--- llvm/include/llvm/TargetParser/Triple.h
+++ llvm/include/llvm/TargetParser/Triple.h
@@ -270,7 +270,9 @@
 Callable,
 Mesh,
 Amplification,
+
 OpenHOS,
+
 LastEnvironmentType = OpenHOS
   };
   enum ObjectFormatType {
@@ -755,6 +757,22 @@
 return getArch() == Triple::dxil;
   }
 
+  bool isShaderModelOS() const {
+return getOS() == Triple::ShaderModel;
+  }
+
+  bool isShaderStageEnvironment() const {
+EnvironmentType Env = getEnvironment();
+return Env == Triple::Pixel || Env == Triple::Vertex ||
+   Env == Triple::Geometry || Env == Triple::Hull ||
+   Env == Triple::Domain || Env == Triple::Compute ||
+   Env == Triple::Library || Env == Triple::RayGeneration ||
+   Env == Triple::Intersection || Env == Triple::AnyHit ||
+   Env == Triple::ClosestHit || Env == Triple::Miss ||
+   Env == Triple::Callable || Env == Triple::Mesh ||
+   Env == Triple::Amplification;
+  }
+
   /// Tests whether the target is SPIR (32- or 64-bit).
   bool isSPIR() const {
 return getArch() == Triple::spir || getArch() == Triple::spir64;
Index: clang/test/Driver/hlsl-lang-targets.hlsl
===
--- clang/test/Driver/hlsl-lang-targets.hlsl
+++ clang/test/Driver/hlsl-lang-targets.hlsl
@@ -1,14 +1,52 @@
-// RUN: not %clang -target x86_64-unknown-unknown %s 2>&1 | FileCheck %s --check-prefix=X86
-// RUN: not %clang -target dxil-unknown-unknown %s 2>&1 | FileCheck %s --check-prefix=DXIL
-// RUN: not %clang -target x86_64-unknown-shadermodel %s 2>&1 | FileCheck %s --check-prefix=SM
-// RUN: not %clang -target spirv64-unknown-unknown %s 2>&1 | FileCheck %s --check-prefix=SPIRV
+// Supported targets
+//
+// RUN: %clang -target dxil--shadermodel6.2-pixel %s -S -o /dev/null 2>&1 | FileCheck --check-prefix=CHECK-VALID %s
+// RUN: %clang -target dxil-unknown-shadermodel6.2-pixel %s -S -o /dev/null 2>&1 | FileCheck --check-prefix=CHECK-VALID %s
+// RUN: %clang -target dxil--shadermodel6.2-library %s -S -o /dev/null 2>&1 | FileCheck --check-prefix=CHECK-VALID %s
+// RUN: %clang -target dxil-unknown-shadermodel6.2-library %s -S -o /dev/null 2>&1 | FileCheck --check-prefix=CHECK-VALID %s
 
+// Empty shader model
+//
+// RUN: not %clang -target dxil %s -S -o /dev/null 2>&1 | FileCheck --check-prefix=CHECK-NO-OS %s
 
-// A completely unsupported target...
-// X86: error: HLSL code generation is unsupported for target 'x86_64-unknown-unknown'
+// Invalid shader models
+//
+// RUN: not %clang -target dxil--linux %s -S -o /dev/null 2>&1 | FileCheck --check-prefix=CHECK-BAD-OS %s
+// RUN: not %clang -target dxil--win32 %s -S -o /dev/null 2>&1 | FileCheck --check-prefix=CHECK-BAD-OS %s
+// RUN: not %clang -target dxil--unknown %s -S -o /dev/null 2>&1 | FileCheck --check-prefix=CHECK-BAD-OS %s
+// RUN: not %clang -target dxil--invalidos %s -S -o /dev/null 2>&1 | FileCheck --check-prefix=CHECK-BAD-OS %s
 
-// Poorly specified targets
-// DXIL: error: HLSL code generation is unsupported for target 'dxil-unknown-unknown'
-// SM: error: HLSL code generation is unsupported for target 'x86_64-unknown-shadermodel'
+// Bad shader model versions. Currently we just check for any version at all.
+//
+// RUN: not %clang -target dxil--shadermodel %s -S -o /dev/null 2>&1 | FileCheck --check-prefix=CHECK-BAD-OS %s
+// RUN: not %clang -target dxil--shadermodel0.0 %s -S -o /dev/null 2>&1 | FileCheck --check-prefix=CHECK-BAD-OS %s
 
-// FIXME// SPIRV: error: HLSL code generation is unsupported for target 'spirv64-unknown-unknown'
+// Empty shader stage
+//
+// RUN: not %clang -target dxil-shadermodel6.2 %s -S -o /dev/null 2>&1 | FileCheck --check-prefix=CHECK-NO-ENV %s
+// RUN: not %clang -target dxil--shadermodel6.2 %s -S -o /dev/null 2>&1 | FileCheck --check-prefix=CHECK-NO-ENV %s
+// RUN: not %clang -target dxil--shadermodel6.2 %s -S -o /dev/null 2>&1 | FileCheck --check-prefix=CHECK-NO-ENV %s
+
+// Invalid shader stages
+//
+// RUN: not %clang -target dxil--shadermodel6.2-unknown %s -S -o /dev/null 2>&1 | FileCheck --check-prefix=CHECK-BAD-ENV %s
+// RUN: not %clang -target dxil--shadermodel6.2-invalidenvironment %s -S -o /dev/null 2>&1 | FileCheck --check-prefix=CHECK-BAD-ENV %s
+// RUN: not %clang -target dxil--shadermodel6.2-eabi %s -S -o /dev/null 2>&1 | FileCheck 

[PATCH] D159103: [Driver][HLSL] Improve diagnostics for invalid shader model and stage

[Justin Bogner via Phabricator via cfe-commits]

bogner added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticDriverKinds.td:710-716
+  "Shader model is required in target '%0' for DXIL generation">;
+def err_drv_dxil_unsupported_shader_model : Error<
+  "Shader model '%0' in target '%1' is invalid for DXIL generation">;
+def err_drv_dxil_missing_shader_stage : Error<
+  "Shader stage is required in target '%0' for DXIL generation">;
+def err_drv_dxil_unsupported_shader_stage : Error<
+  "Shader stage '%0' in target '%1' is invalid for DXIL generation">;

aaron.ballman wrote:
> Hmmm in theory:
> ```
> def err_drv_dxil_bad_shader_required_in_target : Error<
>   "shader %select{model|stage}0 is required in target '%1' for DXIL 
> generation">;
> 
> def err_drv_dxil_bad_shader_unsupported : Error<
>   "shader %select{model|stage}0 '%1' in target '%2' is invalid for DXIL 
> generation">;
> ```
To make that readable you kind of have to do something like this:

```
if (T.isDXIL()) {
  enum { ShaderModel, ShaderStage };
  if (T.getOSName().empty()) {
Diags.Report(diag::err_drv_dxil_bad_shader_required_in_target)
<< ShaderModel << T.str();
  } else if (!T.isShaderModelOS() || T.getOSVersion() == VersionTuple(0)) {
Diags.Report(diag::err_drv_dxil_bad_shader_invalid)
ShaderModel << T.getOSName() << T.str();
  } else if (T.getEnvironmentName().empty()) {
Diags.Report(diag::err_drv_dxil_bad_shader_required_in_target)
<< ShaderStage << T.str();
  } else if (!T.isShaderStageEnvironment()) {
Diags.Report(diag::err_drv_dxil_bad_shader_invalid)
<< ShaderStage << T.getEnvironmentName() << T.str();
  }
}
```

It's not really clear to me which way is better. WDYT?




Comment at: clang/lib/Frontend/CompilerInvocation.cpp:4054
+  if (T.getOSName().empty()) {
+Diags.Report(diag::err_drv_dxil_missing_shader_model) << T.str();
+  } else if (!T.isShaderModelOS() || T.getOSVersion() == VersionTuple(0)) {

aaron.ballman wrote:
> Idle question: can you pass `T` directly here? (I thought we had diagnostic 
> streaming support for `Triple` but I'm not spotting it with a quick look 
> through the source).
Tried it, and no. Looks like we don't have that overload.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D159103/new/

https://reviews.llvm.org/D159103

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158803: [Sema][HLSL] Consolidate handling of HLSL attributes

[Justin Bogner via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGef2b1700f464: [Sema][HLSL] Consolidate handling of HLSL 
attributes (authored by bogner).

Changed prior to commit:
  https://reviews.llvm.org/D158803?vs=554044=554379#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158803/new/

https://reviews.llvm.org/D158803

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/CodeGenHLSL/GlobalDestructors.hlsl
  clang/test/SemaHLSL/Semantics/entry_parameter.hlsl
  clang/test/SemaHLSL/Semantics/groupindex.hlsl
  clang/test/SemaHLSL/entry.hlsl
  clang/test/SemaHLSL/entry_shader_redecl.hlsl
  clang/test/SemaHLSL/num_threads.hlsl

Index: clang/test/SemaHLSL/num_threads.hlsl
===
--- clang/test/SemaHLSL/num_threads.hlsl
+++ clang/test/SemaHLSL/num_threads.hlsl
@@ -1,7 +1,7 @@
-// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-compute -x hlsl -ast-dump -o - %s | FileCheck %s 
-// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-mesh -x hlsl -ast-dump -o - %s | FileCheck %s 
-// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-amplification -x hlsl -ast-dump -o - %s | FileCheck %s 
-// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-library -x hlsl -ast-dump -o - %s | FileCheck %s 
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-compute -x hlsl -ast-dump -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-mesh -x hlsl -ast-dump -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-amplification -x hlsl -ast-dump -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-library -x hlsl -ast-dump -o - %s | FileCheck %s
 // RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-pixel -x hlsl -ast-dump -o - %s -verify
 // RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-vertex -x hlsl -ast-dump -o - %s -verify
 // RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-hull -x hlsl -ast-dump -o - %s -verify
@@ -97,14 +97,20 @@
   return 1;
 }
 
+[numthreads(4,2,1)]
+// CHECK: HLSLNumThreadsAttr 0x{{[0-9a-fA-F]+}}  4 2 1
+int onlyOnForwardDecl();
+
+// CHECK: HLSLNumThreadsAttr 0x{{[0-9a-fA-F]+}}  Inherited 4 2 1
+int onlyOnForwardDecl() {
+  return 1;
+}
 
 #else // Vertex and Pixel only beyond here
-// expected-error-re@+1 {{attribute 'numthreads' is unsupported in {{[A-Za-z]+}} shaders, requires Compute, Amplification, Mesh or Library}}
+// expected-error-re@+1 {{attribute 'numthreads' is unsupported in '{{[A-Za-z]+}}' shaders, requires one of the following: compute, amplification, mesh}}
 [numthreads(1,1,1)]
 int main() {
  return 1;
 }
 
 #endif
-
-
Index: clang/test/SemaHLSL/entry_shader_redecl.hlsl
===
--- /dev/null
+++ clang/test/SemaHLSL/entry_shader_redecl.hlsl
@@ -0,0 +1,74 @@
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-compute -x hlsl -hlsl-entry cs1 -o - %s -ast-dump -verify | FileCheck -DSHADERFN=cs1 -check-prefix=CHECK-ENV %s
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-compute -x hlsl -hlsl-entry cs2 -o - %s -ast-dump -verify | FileCheck -DSHADERFN=cs2 -check-prefix=CHECK-ENV %s
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-compute -x hlsl -hlsl-entry cs3 -o - %s -ast-dump -verify | FileCheck -DSHADERFN=cs3 -check-prefix=CHECK-ENV %s
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library -x hlsl -o - %s -ast-dump -verify | FileCheck -check-prefix=CHECK-LIB %s
+
+// expected-no-diagnostics
+
+// CHECK-ENV: FunctionDecl [[PROTO:0x[0-9a-f]+]] {{.*}} [[SHADERFN]] 'void ()'
+// CHECK-ENV: FunctionDecl 0x{{.*}} prev [[PROTO]] {{.*}} [[SHADERFN]] 'void ()'
+// CHECK-ENV-NEXT: CompoundStmt 0x
+// CHECK-ENV-NEXT: HLSLNumThreadsAttr 0x
+// CHECK-ENV-NEXT: HLSLShaderAttr 0x{{.*}} Implicit Compute
+void cs1();
+[numthreads(1,1,1)] void cs1() {}
+[numthreads(1,1,1)] void cs2();
+void cs2() {}
+[numthreads(1,1,1)] void cs3();
+[numthreads(1,1,1)] void cs3() {}
+
+// CHECK-LIB: FunctionDecl [[PROTO:0x[0-9a-f]+]] {{.*}} s1 'void ()'
+// CHECK-LIB: FunctionDecl 0x{{.*}} prev [[PROTO]] {{.*}} s1 'void ()'
+// CHECK-LIB-NEXT: CompoundStmt 0x
+// CHECK-LIB-NEXT: HLSLShaderAttr 0x{{.*}} Compute
+// CHECK-LIB-NEXT: HLSLNumThreadsAttr 0x
+void s1();
+[shader("compute"), numthreads(1,1,1)] void s1() {}
+
+// CHECK-LIB: FunctionDecl [[PROTO:0x[0-9a-f]+]] {{.*}} s2 'void ()'
+// CHECK-LIB: FunctionDecl 0x{{.*}} prev [[PROTO]] {{.*}} s2 'void ()'
+// CHECK-LIB-NEXT: CompoundStmt 0x
+// CHECK-LIB-NEXT: HLSLShaderAttr 0x{{.*}} Compute
+// CHECK-LIB-NEXT: HLSLNumThreadsAttr 0x
+[shader("compute")] void s2();
+[shader("compute"), numthreads(1,1,1)] void s2() {}
+
+// CHECK-LIB: FunctionDecl [[PROTO:0x[0-9a-f]+]] {{.*}} s3 'void ()'
+// CHECK-LIB: FunctionDecl 0x{{.*}} prev [[PROTO]] {{.*}} s3 'void ()'
+// CHECK-LIB-NEXT: 

[PATCH] D158803: [Sema][HLSL] Consolidate handling of HLSL attributes

[Justin Bogner via Phabricator via cfe-commits]

bogner added inline comments.



Comment at: clang/lib/Sema/SemaDecl.cpp:12395-12398
+  // TODO: This should probably just be llvm_unreachable and we should
+  // reject triples with random ABIs and such when we build the target.
+  // For now, crash.
+  llvm::report_fatal_error("Unhandled environment in triple");

aaron.ballman wrote:
> bogner wrote:
> > aaron.ballman wrote:
> > > Hmmm, is this going to be done as a follow-up relatively soon? 
> > > `report_fatal_error` isn't acceptable in lieu of actual diagnostic 
> > > reporting, so this should either be fixed in this patch or Really Soon 
> > > After™ it lands.
> > Yeah, I'm happy to go and tackle that this week. Really I want an 
> > assert/unreachable here but since it is in fact reachable at the moment 
> > this seemed better as a temporary fix. Note that before this patch we just 
> > index out of bounds of an array and get all of the fun benefits of UB.
> SGTM to do it as a follow-up -- this is better than the out of bounds 
> indexing!
Follow up is here: https://reviews.llvm.org/D159103


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158803/new/

https://reviews.llvm.org/D158803

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D159103: [Driver][HLSL] Improve diagnostics for invalid shader model and stage

[Justin Bogner via Phabricator via cfe-commits]

bogner created this revision.
bogner added reviewers: aaron.ballman, beanz.
Herald added subscribers: Anastasia, mcrosier.
Herald added a project: All.
bogner requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

This adds more validation that a dxil triple is actually useable when
compiling HLSL.

The OS field of the triple needs to be a versioned shader model.
Later, we should set a default if this is empty and check that the
version is a shader model we can actually handle.

The Environment field of the triple needs to be specified and be a
valid shader stage. I'd like to allow this to be empty and treat it
like library, but allowing that currently crashes in DXIL metadata
handling.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D159103

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/test/Driver/hlsl-lang-targets.hlsl
  llvm/include/llvm/TargetParser/Triple.h

Index: llvm/include/llvm/TargetParser/Triple.h
===
--- llvm/include/llvm/TargetParser/Triple.h
+++ llvm/include/llvm/TargetParser/Triple.h
@@ -271,7 +271,9 @@
 Callable,
 Mesh,
 Amplification,
+
 OpenHOS,
+
 LastEnvironmentType = OpenHOS
   };
   enum ObjectFormatType {
@@ -756,6 +758,22 @@
 return getArch() == Triple::dxil;
   }
 
+  bool isShaderModelOS() const {
+return getOS() == Triple::ShaderModel;
+  }
+
+  bool isShaderStageEnvironment() const {
+EnvironmentType Env = getEnvironment();
+return Env == Triple::Pixel || Env == Triple::Vertex ||
+   Env == Triple::Geometry || Env == Triple::Hull ||
+   Env == Triple::Domain || Env == Triple::Compute ||
+   Env == Triple::Library || Env == Triple::RayGeneration ||
+   Env == Triple::Intersection || Env == Triple::AnyHit ||
+   Env == Triple::ClosestHit || Env == Triple::Miss ||
+   Env == Triple::Callable || Env == Triple::Mesh ||
+   Env == Triple::Amplification;
+  }
+
   /// Tests whether the target is SPIR (32- or 64-bit).
   bool isSPIR() const {
 return getArch() == Triple::spir || getArch() == Triple::spir64;
Index: clang/test/Driver/hlsl-lang-targets.hlsl
===
--- clang/test/Driver/hlsl-lang-targets.hlsl
+++ clang/test/Driver/hlsl-lang-targets.hlsl
@@ -1,14 +1,52 @@
-// RUN: not %clang -target x86_64-unknown-unknown %s 2>&1 | FileCheck %s --check-prefix=X86
-// RUN: not %clang -target dxil-unknown-unknown %s 2>&1 | FileCheck %s --check-prefix=DXIL
-// RUN: not %clang -target x86_64-unknown-shadermodel %s 2>&1 | FileCheck %s --check-prefix=SM
-// RUN: not %clang -target spirv64-unknown-unknown %s 2>&1 | FileCheck %s --check-prefix=SPIRV
+// Supported targets
+//
+// RUN: %clang -target dxil--shadermodel6.2-pixel %s -S -o /dev/null 2>&1 | FileCheck --check-prefix=CHECK-VALID %s
+// RUN: %clang -target dxil-unknown-shadermodel6.2-pixel %s -S -o /dev/null 2>&1 | FileCheck --check-prefix=CHECK-VALID %s
+// RUN: %clang -target dxil--shadermodel6.2-library %s -S -o /dev/null 2>&1 | FileCheck --check-prefix=CHECK-VALID %s
+// RUN: %clang -target dxil-unknown-shadermodel6.2-library %s -S -o /dev/null 2>&1 | FileCheck --check-prefix=CHECK-VALID %s
 
+// Empty shader model
+//
+// RUN: not %clang -target dxil %s -S -o /dev/null 2>&1 | FileCheck --check-prefix=CHECK-NO-OS %s
 
-// A completely unsupported target...
-// X86: error: HLSL code generation is unsupported for target 'x86_64-unknown-unknown'
+// Invalid shader models
+//
+// RUN: not %clang -target dxil--linux %s -S -o /dev/null 2>&1 | FileCheck --check-prefix=CHECK-BAD-OS %s
+// RUN: not %clang -target dxil--win32 %s -S -o /dev/null 2>&1 | FileCheck --check-prefix=CHECK-BAD-OS %s
+// RUN: not %clang -target dxil--unknown %s -S -o /dev/null 2>&1 | FileCheck --check-prefix=CHECK-BAD-OS %s
+// RUN: not %clang -target dxil--invalidos %s -S -o /dev/null 2>&1 | FileCheck --check-prefix=CHECK-BAD-OS %s
 
-// Poorly specified targets
-// DXIL: error: HLSL code generation is unsupported for target 'dxil-unknown-unknown'
-// SM: error: HLSL code generation is unsupported for target 'x86_64-unknown-shadermodel'
+// Bad shader model versions. Currently we just check for any version at all.
+//
+// RUN: not %clang -target dxil--shadermodel %s -S -o /dev/null 2>&1 | FileCheck --check-prefix=CHECK-BAD-OS %s
+// RUN: not %clang -target dxil--shadermodel0.0 %s -S -o /dev/null 2>&1 | FileCheck --check-prefix=CHECK-BAD-OS %s
 
-// FIXME// SPIRV: error: HLSL code generation is unsupported for target 'spirv64-unknown-unknown'
+// Empty shader stage
+//
+// RUN: not %clang -target dxil-shadermodel6.2 %s -S -o /dev/null 2>&1 | FileCheck --check-prefix=CHECK-NO-ENV %s
+// RUN: not %clang -target dxil--shadermodel6.2 %s -S -o /dev/null 2>&1 | FileCheck 

[PATCH] D158803: [Sema][HLSL] Consolidate handling of HLSL attributes

[Justin Bogner via Phabricator via cfe-commits]

bogner updated this revision to Diff 554044.
bogner added a comment.

- Now with more const
- Added DiagnoseHLSLAttrStageMismatch and avoided using raw strings in 
diagnostics


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158803/new/

https://reviews.llvm.org/D158803

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/CodeGenHLSL/GlobalDestructors.hlsl
  clang/test/SemaHLSL/Semantics/entry_parameter.hlsl
  clang/test/SemaHLSL/Semantics/groupindex.hlsl
  clang/test/SemaHLSL/entry.hlsl
  clang/test/SemaHLSL/entry_shader_redecl.hlsl
  clang/test/SemaHLSL/num_threads.hlsl

Index: clang/test/SemaHLSL/num_threads.hlsl
===
--- clang/test/SemaHLSL/num_threads.hlsl
+++ clang/test/SemaHLSL/num_threads.hlsl
@@ -1,7 +1,7 @@
-// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-compute -x hlsl -ast-dump -o - %s | FileCheck %s 
-// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-mesh -x hlsl -ast-dump -o - %s | FileCheck %s 
-// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-amplification -x hlsl -ast-dump -o - %s | FileCheck %s 
-// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-library -x hlsl -ast-dump -o - %s | FileCheck %s 
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-compute -x hlsl -ast-dump -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-mesh -x hlsl -ast-dump -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-amplification -x hlsl -ast-dump -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-library -x hlsl -ast-dump -o - %s | FileCheck %s
 // RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-pixel -x hlsl -ast-dump -o - %s -verify
 // RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-vertex -x hlsl -ast-dump -o - %s -verify
 // RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-hull -x hlsl -ast-dump -o - %s -verify
@@ -97,14 +97,20 @@
   return 1;
 }
 
+[numthreads(4,2,1)]
+// CHECK: HLSLNumThreadsAttr 0x{{[0-9a-fA-F]+}}  4 2 1
+int onlyOnForwardDecl();
+
+// CHECK: HLSLNumThreadsAttr 0x{{[0-9a-fA-F]+}}  Inherited 4 2 1
+int onlyOnForwardDecl() {
+  return 1;
+}
 
 #else // Vertex and Pixel only beyond here
-// expected-error-re@+1 {{attribute 'numthreads' is unsupported in {{[A-Za-z]+}} shaders, requires Compute, Amplification, Mesh or Library}}
+// expected-error-re@+1 {{attribute 'numthreads' is unsupported in '{{[A-Za-z]+}}' shaders, requires one of the following: compute, amplification, mesh}}
 [numthreads(1,1,1)]
 int main() {
  return 1;
 }
 
 #endif
-
-
Index: clang/test/SemaHLSL/entry_shader_redecl.hlsl
===
--- /dev/null
+++ clang/test/SemaHLSL/entry_shader_redecl.hlsl
@@ -0,0 +1,75 @@
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-compute -x hlsl -hlsl-entry cs1 -o - %s -ast-dump -verify | FileCheck -DSHADERFN=cs1 -check-prefix=CHECK-ENV %s
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-compute -x hlsl -hlsl-entry cs2 -o - %s -ast-dump -verify | FileCheck -DSHADERFN=cs2 -check-prefix=CHECK-ENV %s
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-compute -x hlsl -hlsl-entry cs3 -o - %s -ast-dump -verify | FileCheck -DSHADERFN=cs3 -check-prefix=CHECK-ENV %s
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library -x hlsl -o - %s -ast-dump -verify | FileCheck -check-prefix=CHECK-LIB %s
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3 -x hlsl -o - %s -ast-dump -verify | FileCheck -check-prefix=CHECK-LIB %s
+
+// expected-no-diagnostics
+
+// CHECK-ENV: FunctionDecl [[PROTO:0x[0-9a-f]+]] {{.*}} [[SHADERFN]] 'void ()'
+// CHECK-ENV: FunctionDecl 0x{{.*}} prev [[PROTO]] {{.*}} [[SHADERFN]] 'void ()'
+// CHECK-ENV-NEXT: CompoundStmt 0x
+// CHECK-ENV-NEXT: HLSLNumThreadsAttr 0x
+// CHECK-ENV-NEXT: HLSLShaderAttr 0x{{.*}} Implicit Compute
+void cs1();
+[numthreads(1,1,1)] void cs1() {}
+[numthreads(1,1,1)] void cs2();
+void cs2() {}
+[numthreads(1,1,1)] void cs3();
+[numthreads(1,1,1)] void cs3() {}
+
+// CHECK-LIB: FunctionDecl [[PROTO:0x[0-9a-f]+]] {{.*}} s1 'void ()'
+// CHECK-LIB: FunctionDecl 0x{{.*}} prev [[PROTO]] {{.*}} s1 'void ()'
+// CHECK-LIB-NEXT: CompoundStmt 0x
+// CHECK-LIB-NEXT: HLSLShaderAttr 0x{{.*}} Compute
+// CHECK-LIB-NEXT: HLSLNumThreadsAttr 0x
+void s1();
+[shader("compute"), numthreads(1,1,1)] void s1() {}
+
+// CHECK-LIB: FunctionDecl [[PROTO:0x[0-9a-f]+]] {{.*}} s2 'void ()'
+// CHECK-LIB: FunctionDecl 0x{{.*}} prev [[PROTO]] {{.*}} s2 'void ()'
+// CHECK-LIB-NEXT: CompoundStmt 0x
+// CHECK-LIB-NEXT: HLSLShaderAttr 0x{{.*}} Compute
+// CHECK-LIB-NEXT: HLSLNumThreadsAttr 0x
+[shader("compute")] void s2();
+[shader("compute"), numthreads(1,1,1)] void s2() {}
+
+// CHECK-LIB: FunctionDecl [[PROTO:0x[0-9a-f]+]] {{.*}} s3 'void ()'
+// CHECK-LIB: FunctionDecl 0x{{.*}} prev [[PROTO]] 

[PATCH] D158803: [Sema][HLSL] Consolidate handling of HLSL attributes

[Justin Bogner via Phabricator via cfe-commits]

bogner added inline comments.



Comment at: clang/lib/Sema/SemaDecl.cpp:12395-12398
+  // TODO: This should probably just be llvm_unreachable and we should
+  // reject triples with random ABIs and such when we build the target.
+  // For now, crash.
+  llvm::report_fatal_error("Unhandled environment in triple");

aaron.ballman wrote:
> Hmmm, is this going to be done as a follow-up relatively soon? 
> `report_fatal_error` isn't acceptable in lieu of actual diagnostic reporting, 
> so this should either be fixed in this patch or Really Soon After™ it lands.
Yeah, I'm happy to go and tackle that this week. Really I want an 
assert/unreachable here but since it is in fact reachable at the moment this 
seemed better as a temporary fix. Note that before this patch we just index out 
of bounds of an array and get all of the fun benefits of UB.



Comment at: clang/lib/Sema/SemaDecl.cpp:12423
+  << NT << HLSLShaderAttr::ConvertShaderTypeToStr(ST)
+  << "compute, amplification, or mesh";
+  FD->setInvalidDecl();

aaron.ballman wrote:
> This should be part of the diagnostic wording itself (using `%select`) rather 
> than passed in as a string argument. The same suggestion applies elsewhere -- 
> we want to keep English strings out of the streaming arguments as much as 
> possible to make localization efforts possible.
That's a good point, though in this case the words we want are the value to the 
`compute(...)` attribute or part of the triple, so translating them wouldn't 
really make sense. Would it be better to do something using 
`ConvertShaderTypeToStr`? Also I'll probably update the error message text to 
quote the other use of ConvertShaderTypeToStr so it's clear that it isn't just 
a random word.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158803/new/

https://reviews.llvm.org/D158803

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158803: [Sema][HLSL] Consolidate handling of HLSL attributes

[Justin Bogner via Phabricator via cfe-commits]

bogner updated this revision to Diff 553964.
bogner added a comment.

Rebase


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158803/new/

https://reviews.llvm.org/D158803

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/CodeGenHLSL/GlobalDestructors.hlsl
  clang/test/SemaHLSL/Semantics/entry_parameter.hlsl
  clang/test/SemaHLSL/Semantics/groupindex.hlsl
  clang/test/SemaHLSL/entry.hlsl
  clang/test/SemaHLSL/entry_shader_redecl.hlsl
  clang/test/SemaHLSL/num_threads.hlsl

Index: clang/test/SemaHLSL/num_threads.hlsl
===
--- clang/test/SemaHLSL/num_threads.hlsl
+++ clang/test/SemaHLSL/num_threads.hlsl
@@ -1,7 +1,7 @@
-// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-compute -x hlsl -ast-dump -o - %s | FileCheck %s 
-// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-mesh -x hlsl -ast-dump -o - %s | FileCheck %s 
-// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-amplification -x hlsl -ast-dump -o - %s | FileCheck %s 
-// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-library -x hlsl -ast-dump -o - %s | FileCheck %s 
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-compute -x hlsl -ast-dump -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-mesh -x hlsl -ast-dump -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-amplification -x hlsl -ast-dump -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-library -x hlsl -ast-dump -o - %s | FileCheck %s
 // RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-pixel -x hlsl -ast-dump -o - %s -verify
 // RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-vertex -x hlsl -ast-dump -o - %s -verify
 // RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-hull -x hlsl -ast-dump -o - %s -verify
@@ -97,14 +97,20 @@
   return 1;
 }
 
+[numthreads(4,2,1)]
+// CHECK: HLSLNumThreadsAttr 0x{{[0-9a-fA-F]+}}  4 2 1
+int onlyOnForwardDecl();
+
+// CHECK: HLSLNumThreadsAttr 0x{{[0-9a-fA-F]+}}  Inherited 4 2 1
+int onlyOnForwardDecl() {
+  return 1;
+}
 
 #else // Vertex and Pixel only beyond here
-// expected-error-re@+1 {{attribute 'numthreads' is unsupported in {{[A-Za-z]+}} shaders, requires Compute, Amplification, Mesh or Library}}
+// expected-error-re@+1 {{attribute 'numthreads' is unsupported in {{[A-Za-z]+}} shaders, requires compute, amplification, or mesh}}
 [numthreads(1,1,1)]
 int main() {
  return 1;
 }
 
 #endif
-
-
Index: clang/test/SemaHLSL/entry_shader_redecl.hlsl
===
--- /dev/null
+++ clang/test/SemaHLSL/entry_shader_redecl.hlsl
@@ -0,0 +1,75 @@
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-compute -x hlsl -hlsl-entry cs1 -o - %s -ast-dump -verify | FileCheck -DSHADERFN=cs1 -check-prefix=CHECK-ENV %s
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-compute -x hlsl -hlsl-entry cs2 -o - %s -ast-dump -verify | FileCheck -DSHADERFN=cs2 -check-prefix=CHECK-ENV %s
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-compute -x hlsl -hlsl-entry cs3 -o - %s -ast-dump -verify | FileCheck -DSHADERFN=cs3 -check-prefix=CHECK-ENV %s
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library -x hlsl -o - %s -ast-dump -verify | FileCheck -check-prefix=CHECK-LIB %s
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3 -x hlsl -o - %s -ast-dump -verify | FileCheck -check-prefix=CHECK-LIB %s
+
+// expected-no-diagnostics
+
+// CHECK-ENV: FunctionDecl [[PROTO:0x[0-9a-f]+]] {{.*}} [[SHADERFN]] 'void ()'
+// CHECK-ENV: FunctionDecl 0x{{.*}} prev [[PROTO]] {{.*}} [[SHADERFN]] 'void ()'
+// CHECK-ENV-NEXT: CompoundStmt 0x
+// CHECK-ENV-NEXT: HLSLNumThreadsAttr 0x
+// CHECK-ENV-NEXT: HLSLShaderAttr 0x{{.*}} Implicit Compute
+void cs1();
+[numthreads(1,1,1)] void cs1() {}
+[numthreads(1,1,1)] void cs2();
+void cs2() {}
+[numthreads(1,1,1)] void cs3();
+[numthreads(1,1,1)] void cs3() {}
+
+// CHECK-LIB: FunctionDecl [[PROTO:0x[0-9a-f]+]] {{.*}} s1 'void ()'
+// CHECK-LIB: FunctionDecl 0x{{.*}} prev [[PROTO]] {{.*}} s1 'void ()'
+// CHECK-LIB-NEXT: CompoundStmt 0x
+// CHECK-LIB-NEXT: HLSLShaderAttr 0x{{.*}} Compute
+// CHECK-LIB-NEXT: HLSLNumThreadsAttr 0x
+void s1();
+[shader("compute"), numthreads(1,1,1)] void s1() {}
+
+// CHECK-LIB: FunctionDecl [[PROTO:0x[0-9a-f]+]] {{.*}} s2 'void ()'
+// CHECK-LIB: FunctionDecl 0x{{.*}} prev [[PROTO]] {{.*}} s2 'void ()'
+// CHECK-LIB-NEXT: CompoundStmt 0x
+// CHECK-LIB-NEXT: HLSLShaderAttr 0x{{.*}} Compute
+// CHECK-LIB-NEXT: HLSLNumThreadsAttr 0x
+[shader("compute")] void s2();
+[shader("compute"), numthreads(1,1,1)] void s2() {}
+
+// CHECK-LIB: FunctionDecl [[PROTO:0x[0-9a-f]+]] {{.*}} s3 'void ()'
+// CHECK-LIB: FunctionDecl 0x{{.*}} prev [[PROTO]] {{.*}} s3 'void ()'
+// CHECK-LIB-NEXT: CompoundStmt 0x
+// CHECK-LIB-NEXT: HLSLShaderAttr 0x{{.*}} Compute
+// 

[PATCH] D158820: [Sema][HLSL] Fix naming of anyhit/closesthit shaders

[Justin Bogner via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG43a9b92d79ab: [Sema][HLSL] Fix naming of anyhit/closesthit 
shaders (authored by bogner).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158820/new/

https://reviews.llvm.org/D158820

Files:
  clang/include/clang/Basic/Attr.td
  clang/test/SemaHLSL/entry_shader.hlsl
  clang/test/SemaHLSL/shader_type_attr.hlsl
  clang/test/SemaHLSL/valid-shader-stages.hlsl

Index: clang/test/SemaHLSL/valid-shader-stages.hlsl
===
--- /dev/null
+++ clang/test/SemaHLSL/valid-shader-stages.hlsl
@@ -0,0 +1,24 @@
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library -x hlsl -finclude-default-header -o - %s -verify
+
+// expected-no-diagnostics
+
+[shader("pixel")] void pixel() {}
+[shader("vertex")] void vertex() {}
+[shader("raygeneration")] void raygeneration() {}
+[shader("intersection")] void intersection() {}
+
+[numthreads(1,1,1)][shader("compute")] void compute() {}
+[numthreads(1,1,1)][shader("mesh")] void mesh() {}
+
+// Note: the rest of these have additional constraints that aren't implemented
+// yet, so here we just declare them to make sure the spelling works and
+// whatnot.
+[shader("geometry")] void geometry();
+[shader("hull")] void hull();
+[shader("domain")] void domain();
+[shader("callable")] void callable();
+[shader("closesthit")] void closesthit();
+[shader("anyhit")] void anyhit();
+[shader("miss")] void miss();
+
+[numthreads(1,1,1)][shader("amplification")] void amplification();
Index: clang/test/SemaHLSL/shader_type_attr.hlsl
===
--- clang/test/SemaHLSL/shader_type_attr.hlsl
+++ clang/test/SemaHLSL/shader_type_attr.hlsl
@@ -28,7 +28,7 @@
 } // namespace spec
 
 // expected-error@+1 {{'shader' attribute parameters do not match the previous declaration}}
-[shader("compute")]
+[shader("pixel")]
 // expected-note@+1 {{conflicting attribute is here}}
 [shader("vertex")]
 int doubledUp() {
@@ -40,7 +40,7 @@
 int forwardDecl();
 
 // expected-error@+1 {{'shader' attribute parameters do not match the previous declaration}}
-[shader("compute")]
+[shader("compute")][numthreads(8,1,1)]
 int forwardDecl() {
   return 1;
 }
@@ -57,19 +57,22 @@
 [shader("library")]
 #endif // END of FAIL
 
-// CHECK:HLSLShaderAttr 0x{{[0-9a-fA-F]+}}  Compute
-[shader("compute")]
+// CHECK:HLSLShaderAttr 0x{{[0-9a-fA-F]+}}  Compute
+// CHECK:HLSLNumThreadsAttr 0x{{[0-9a-fA-F]+}}  8 1 1
+[shader("compute")][numthreads(8,1,1)]
 int entry() {
   return 1;
 }
 
 // Because these two attributes match, they should both appear in the AST
-[shader("compute")]
-// CHECK:HLSLShaderAttr 0x{{[0-9a-fA-F]+}}  Compute
+[shader("compute")][numthreads(8,1,1)]
+// CHECK:HLSLShaderAttr 0x{{[0-9a-fA-F]+}}  Compute
+// CHECK:HLSLNumThreadsAttr 0x{{[0-9a-fA-F]+}}  8 1 1
 int secondFn();
 
-[shader("compute")]
-// CHECK:HLSLShaderAttr 0x{{[0-9a-fA-F]+}}  Compute
+[shader("compute")][numthreads(8,1,1)]
+// CHECK:HLSLShaderAttr 0x{{[0-9a-fA-F]+}}  Compute
+// CHECK:HLSLNumThreadsAttr 0x{{[0-9a-fA-F]+}}  8 1 1
 int secondFn() {
   return 1;
 }
Index: clang/test/SemaHLSL/entry_shader.hlsl
===
--- clang/test/SemaHLSL/entry_shader.hlsl
+++ clang/test/SemaHLSL/entry_shader.hlsl
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-compute -x hlsl -hlsl-entry foo  -o - %s -DSHADER='"anyHit"' -verify
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-compute -x hlsl -hlsl-entry foo  -o - %s -DSHADER='"mesh"' -verify
 // RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-compute -x hlsl -hlsl-entry foo  -o - %s -DSHADER='"compute"'
 
 // expected-error@+1 {{'shader' attribute on entry function does not match the target profile}}
Index: clang/include/clang/Basic/Attr.td
===
--- clang/include/clang/Basic/Attr.td
+++ clang/include/clang/Basic/Attr.td
@@ -4143,8 +4143,8 @@
 EnumArgument<"Type", "ShaderType",
  [
"pixel", "vertex", "geometry", "hull", "domain", "compute",
-   "library", "raygeneration", "intersection", "anyHit",
-   "closestHit", "miss", "callable", "mesh", "amplification"
+   "library", "raygeneration", "intersection", "anyhit",
+   "closesthit", "miss", "callable", "mesh", "amplification"
  ],
  [
"Pixel", "Vertex", "Geometry", "Hull", "Domain", "Compute",
@@ -4232,4 +4232,3 @@
   let Subjects = SubjectList<[TypedefName], ErrorDiag>;
   let Documentation = [Undocumented];
 }
-
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158820: [Sema][HLSL] Fix naming of anyhit/closesthit shaders

[Justin Bogner via Phabricator via cfe-commits]

bogner updated this revision to Diff 553588.
bogner added a comment.

Address feedback and simplify valid shader stage test


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158820/new/

https://reviews.llvm.org/D158820

Files:
  clang/include/clang/Basic/Attr.td
  clang/test/SemaHLSL/entry_shader.hlsl
  clang/test/SemaHLSL/shader_type_attr.hlsl
  clang/test/SemaHLSL/valid-shader-stages.hlsl

Index: clang/test/SemaHLSL/valid-shader-stages.hlsl
===
--- /dev/null
+++ clang/test/SemaHLSL/valid-shader-stages.hlsl
@@ -0,0 +1,24 @@
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library -x hlsl -finclude-default-header -o - %s -verify
+
+// expected-no-diagnostics
+
+[shader("pixel")] void pixel() {}
+[shader("vertex")] void vertex() {}
+[shader("raygeneration")] void raygeneration() {}
+[shader("intersection")] void intersection() {}
+
+[numthreads(1,1,1)][shader("compute")] void compute() {}
+[numthreads(1,1,1)][shader("mesh")] void mesh() {}
+
+// Note: the rest of these have additional constraints that aren't implemented
+// yet, so here we just declare them to make sure the spelling works and
+// whatnot.
+[shader("geometry")] void geometry();
+[shader("hull")] void hull();
+[shader("domain")] void domain();
+[shader("callable")] void callable();
+[shader("closesthit")] void closesthit();
+[shader("anyhit")] void anyhit();
+[shader("miss")] void miss();
+
+[numthreads(1,1,1)][shader("amplification")] void amplification();
Index: clang/test/SemaHLSL/shader_type_attr.hlsl
===
--- clang/test/SemaHLSL/shader_type_attr.hlsl
+++ clang/test/SemaHLSL/shader_type_attr.hlsl
@@ -28,7 +28,7 @@
 } // namespace spec
 
 // expected-error@+1 {{'shader' attribute parameters do not match the previous declaration}}
-[shader("compute")]
+[shader("pixel")]
 // expected-note@+1 {{conflicting attribute is here}}
 [shader("vertex")]
 int doubledUp() {
@@ -40,7 +40,7 @@
 int forwardDecl();
 
 // expected-error@+1 {{'shader' attribute parameters do not match the previous declaration}}
-[shader("compute")]
+[shader("compute")][numthreads(8,1,1)]
 int forwardDecl() {
   return 1;
 }
@@ -57,19 +57,22 @@
 [shader("library")]
 #endif // END of FAIL
 
-// CHECK:HLSLShaderAttr 0x{{[0-9a-fA-F]+}}  Compute
-[shader("compute")]
+// CHECK:HLSLShaderAttr 0x{{[0-9a-fA-F]+}}  Compute
+// CHECK:HLSLNumThreadsAttr 0x{{[0-9a-fA-F]+}}  8 1 1
+[shader("compute")][numthreads(8,1,1)]
 int entry() {
   return 1;
 }
 
 // Because these two attributes match, they should both appear in the AST
-[shader("compute")]
-// CHECK:HLSLShaderAttr 0x{{[0-9a-fA-F]+}}  Compute
+[shader("compute")][numthreads(8,1,1)]
+// CHECK:HLSLShaderAttr 0x{{[0-9a-fA-F]+}}  Compute
+// CHECK:HLSLNumThreadsAttr 0x{{[0-9a-fA-F]+}}  8 1 1
 int secondFn();
 
-[shader("compute")]
-// CHECK:HLSLShaderAttr 0x{{[0-9a-fA-F]+}}  Compute
+[shader("compute")][numthreads(8,1,1)]
+// CHECK:HLSLShaderAttr 0x{{[0-9a-fA-F]+}}  Compute
+// CHECK:HLSLNumThreadsAttr 0x{{[0-9a-fA-F]+}}  8 1 1
 int secondFn() {
   return 1;
 }
Index: clang/test/SemaHLSL/entry_shader.hlsl
===
--- clang/test/SemaHLSL/entry_shader.hlsl
+++ clang/test/SemaHLSL/entry_shader.hlsl
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-compute -x hlsl -hlsl-entry foo  -o - %s -DSHADER='"anyHit"' -verify
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-compute -x hlsl -hlsl-entry foo  -o - %s -DSHADER='"mesh"' -verify
 // RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-compute -x hlsl -hlsl-entry foo  -o - %s -DSHADER='"compute"'
 
 // expected-error@+1 {{'shader' attribute on entry function does not match the target profile}}
Index: clang/include/clang/Basic/Attr.td
===
--- clang/include/clang/Basic/Attr.td
+++ clang/include/clang/Basic/Attr.td
@@ -4143,8 +4143,8 @@
 EnumArgument<"Type", "ShaderType",
  [
"pixel", "vertex", "geometry", "hull", "domain", "compute",
-   "library", "raygeneration", "intersection", "anyHit",
-   "closestHit", "miss", "callable", "mesh", "amplification"
+   "library", "raygeneration", "intersection", "anyhit",
+   "closesthit", "miss", "callable", "mesh", "amplification"
  ],
  [
"Pixel", "Vertex", "Geometry", "Hull", "Domain", "Compute",
@@ -4232,4 +4232,3 @@
   let Subjects = SubjectList<[TypedefName], ErrorDiag>;
   let Documentation = [Undocumented];
 }
-
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158820: [Sema][HLSL] Fix naming of anyhit/closesthit shaders

[Justin Bogner via Phabricator via cfe-commits]

bogner added inline comments.



Comment at: clang/test/SemaHLSL/entry_shader.hlsl:1
-// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-compute -x hlsl -hlsl-entry 
foo  -o - %s -DSHADER='"anyHit"' -verify
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-compute -x hlsl -hlsl-entry 
foo  -o - %s -DSHADER='"mesh"' -verify
 // RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-compute -x hlsl -hlsl-entry 
foo  -o - %s -DSHADER='"compute"'

bob80905 wrote:
> Why was this changed to mesh instead of "anyhit" ?
Another case of trying to keep things to one error - `anyhit` doesn't support 
the `numthreads` attribute



Comment at: clang/test/SemaHLSL/shader_type_attr.hlsl:30
 
 // expected-error@+1 {{'shader' attribute parameters do not match the previous 
declaration}}
+[shader("pixel")]

beanz wrote:
> bob80905 wrote:
> > bogner wrote:
> > > bob80905 wrote:
> > > > I don't think we should expect this error, given that there is no 
> > > > previous declaration for doubledUp(). Maybe something else like %0 and 
> > > > %1 are incompatible attributes? 
> > > I don't necessarily disagree, but if we're going to change that I think 
> > > it should be in a different change than this one.
> > I'm also curious, why is the change from compute to pixel necessary here?
> I suspect this change is just to prevent the presence of the `compute` 
> annotation from triggering the `missing numthreads` error.
That's correct - just trying to keep to testing the conflicting attribute error 
and not have a spurious numthreads as well.



Comment at: clang/test/SemaHLSL/shader_type_attr.hlsl:61
 // CHECK:HLSLShaderAttr 0x{{[0-9a-fA-F]+}}  Compute
-[shader("compute")]
+[shader("compute"), numthreads(8,1,1)]
 int entry() {

beanz wrote:
> Oh interesting! DXC doesn't actually support this syntax, but I think it is 
> worth supporting in Clang.
> Should we also check for the numthreads attribute?
Hm, it might be better to stick to the syntax dxc handles for these tests at 
least. I'll update that. Sure, checking both attributes seems like a good idea.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158820/new/

https://reviews.llvm.org/D158820

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158820: [Sema][HLSL] Fix naming of anyhit/closesthit shaders

[Justin Bogner via Phabricator via cfe-commits]

bogner added inline comments.



Comment at: clang/test/SemaHLSL/shader_type_attr.hlsl:30
 
 // expected-error@+1 {{'shader' attribute parameters do not match the previous 
declaration}}
+[shader("pixel")]

bob80905 wrote:
> I don't think we should expect this error, given that there is no previous 
> declaration for doubledUp(). Maybe something else like %0 and %1 are 
> incompatible attributes? 
I don't necessarily disagree, but if we're going to change that I think it 
should be in a different change than this one.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158820/new/

https://reviews.llvm.org/D158820

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158803: [Sema][HLSL] Consolidate handling of HLSL attributes

[Justin Bogner via Phabricator via cfe-commits]

bogner updated this revision to Diff 553395.
bogner added a comment.

Broke out a couple of semi-unrelated changes


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158803/new/

https://reviews.llvm.org/D158803

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/CodeGenHLSL/GlobalDestructors.hlsl
  clang/test/SemaHLSL/Semantics/entry_parameter.hlsl
  clang/test/SemaHLSL/Semantics/groupindex.hlsl
  clang/test/SemaHLSL/entry.hlsl
  clang/test/SemaHLSL/entry_shader_redecl.hlsl
  clang/test/SemaHLSL/num_threads.hlsl

Index: clang/test/SemaHLSL/num_threads.hlsl
===
--- clang/test/SemaHLSL/num_threads.hlsl
+++ clang/test/SemaHLSL/num_threads.hlsl
@@ -1,7 +1,7 @@
-// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-compute -x hlsl -ast-dump -o - %s | FileCheck %s 
-// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-mesh -x hlsl -ast-dump -o - %s | FileCheck %s 
-// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-amplification -x hlsl -ast-dump -o - %s | FileCheck %s 
-// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-library -x hlsl -ast-dump -o - %s | FileCheck %s 
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-compute -x hlsl -ast-dump -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-mesh -x hlsl -ast-dump -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-amplification -x hlsl -ast-dump -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-library -x hlsl -ast-dump -o - %s | FileCheck %s
 // RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-pixel -x hlsl -ast-dump -o - %s -verify
 // RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-vertex -x hlsl -ast-dump -o - %s -verify
 // RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-hull -x hlsl -ast-dump -o - %s -verify
@@ -97,14 +97,20 @@
   return 1;
 }
 
+[numthreads(4,2,1)]
+// CHECK: HLSLNumThreadsAttr 0x{{[0-9a-fA-F]+}}  4 2 1
+int onlyOnForwardDecl();
+
+// CHECK: HLSLNumThreadsAttr 0x{{[0-9a-fA-F]+}}  Inherited 4 2 1
+int onlyOnForwardDecl() {
+  return 1;
+}
 
 #else // Vertex and Pixel only beyond here
-// expected-error-re@+1 {{attribute 'numthreads' is unsupported in {{[A-Za-z]+}} shaders, requires Compute, Amplification, Mesh or Library}}
+// expected-error-re@+1 {{attribute 'numthreads' is unsupported in {{[A-Za-z]+}} shaders, requires compute, amplification, or mesh}}
 [numthreads(1,1,1)]
 int main() {
  return 1;
 }
 
 #endif
-
-
Index: clang/test/SemaHLSL/entry_shader_redecl.hlsl
===
--- /dev/null
+++ clang/test/SemaHLSL/entry_shader_redecl.hlsl
@@ -0,0 +1,75 @@
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-compute -x hlsl -hlsl-entry cs1 -o - %s -ast-dump -verify | FileCheck -DSHADERFN=cs1 -check-prefix=CHECK-ENV %s
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-compute -x hlsl -hlsl-entry cs2 -o - %s -ast-dump -verify | FileCheck -DSHADERFN=cs2 -check-prefix=CHECK-ENV %s
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-compute -x hlsl -hlsl-entry cs3 -o - %s -ast-dump -verify | FileCheck -DSHADERFN=cs3 -check-prefix=CHECK-ENV %s
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library -x hlsl -o - %s -ast-dump -verify | FileCheck -check-prefix=CHECK-LIB %s
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3 -x hlsl -o - %s -ast-dump -verify | FileCheck -check-prefix=CHECK-LIB %s
+
+// expected-no-diagnostics
+
+// CHECK-ENV: FunctionDecl [[PROTO:0x[0-9a-f]+]] {{.*}} [[SHADERFN]] 'void ()'
+// CHECK-ENV: FunctionDecl 0x{{.*}} prev [[PROTO]] {{.*}} [[SHADERFN]] 'void ()'
+// CHECK-ENV-NEXT: CompoundStmt 0x
+// CHECK-ENV-NEXT: HLSLNumThreadsAttr 0x
+// CHECK-ENV-NEXT: HLSLShaderAttr 0x{{.*}} Implicit Compute
+void cs1();
+[numthreads(1,1,1)] void cs1() {}
+[numthreads(1,1,1)] void cs2();
+void cs2() {}
+[numthreads(1,1,1)] void cs3();
+[numthreads(1,1,1)] void cs3() {}
+
+// CHECK-LIB: FunctionDecl [[PROTO:0x[0-9a-f]+]] {{.*}} s1 'void ()'
+// CHECK-LIB: FunctionDecl 0x{{.*}} prev [[PROTO]] {{.*}} s1 'void ()'
+// CHECK-LIB-NEXT: CompoundStmt 0x
+// CHECK-LIB-NEXT: HLSLShaderAttr 0x{{.*}} Compute
+// CHECK-LIB-NEXT: HLSLNumThreadsAttr 0x
+void s1();
+[shader("compute"), numthreads(1,1,1)] void s1() {}
+
+// CHECK-LIB: FunctionDecl [[PROTO:0x[0-9a-f]+]] {{.*}} s2 'void ()'
+// CHECK-LIB: FunctionDecl 0x{{.*}} prev [[PROTO]] {{.*}} s2 'void ()'
+// CHECK-LIB-NEXT: CompoundStmt 0x
+// CHECK-LIB-NEXT: HLSLShaderAttr 0x{{.*}} Compute
+// CHECK-LIB-NEXT: HLSLNumThreadsAttr 0x
+[shader("compute")] void s2();
+[shader("compute"), numthreads(1,1,1)] void s2() {}
+
+// CHECK-LIB: FunctionDecl [[PROTO:0x[0-9a-f]+]] {{.*}} s3 'void ()'
+// CHECK-LIB: FunctionDecl 0x{{.*}} prev [[PROTO]] {{.*}} s3 'void ()'
+// CHECK-LIB-NEXT: CompoundStmt 0x
+// CHECK-LIB-NEXT: 

[PATCH] D158820: [Sema][HLSL] Fix naming of anyhit/closesthit shaders

[Justin Bogner via Phabricator via cfe-commits]

bogner created this revision.
bogner added reviewers: beanz, python3kgae.
Herald added subscribers: Anastasia, mcrosier.
Herald added a reviewer: aaron.ballman.
Herald added a project: All.
bogner requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Also test all shader types and be a bit more careful to make shaders
reasonably valid in these tests.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D158820

Files:
  clang/include/clang/Basic/Attr.td
  clang/test/SemaHLSL/entry_shader.hlsl
  clang/test/SemaHLSL/shader_type_attr.hlsl
  clang/test/SemaHLSL/valid-shader-stages.hlsl

Index: clang/test/SemaHLSL/valid-shader-stages.hlsl
===
--- /dev/null
+++ clang/test/SemaHLSL/valid-shader-stages.hlsl
@@ -0,0 +1,37 @@
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library -x hlsl -finclude-default-header -o - %s -verify
+
+// expected-no-diagnostics
+
+[shader("pixel")] void pixel();
+[shader("vertex")] void vertex();
+[shader("geometry")] void geometry();
+[shader("hull")] void hull();
+[shader("domain")] void domain();
+[shader("raygeneration")] void raygeneration();
+[shader("intersection")] void intersection();
+[shader("anyhit")] void anyhit();
+[shader("closesthit")] void closesthit();
+[shader("miss")] void miss();
+[shader("callable")] void callable();
+
+[numthreads(1,1,1)][shader("compute")] void compute() {}
+[numthreads(1,1,1)][shader("mesh")] void mesh() {}
+
+#ifndef __DXC_VERSION_MAJOR
+// TODO: These should all come from the implicit header eventually.
+#define inout
+#define in
+struct BuiltInTriangleIntersectionAttributes {
+  float2 barycentrics;
+};
+#endif
+
+typedef BuiltInTriangleIntersectionAttributes Attrs;
+struct Payload {
+  float4 color;
+};
+
+[shader("raygeneration")] void raygeneration() {}
+[shader("closesthit")] void closesthit(inout Payload payload, in Attrs attr) {}
+[shader("anyhit")] void anyhit(inout Payload payload, in Attrs attr) {}
+[shader("miss")] void miss(inout Payload payload) {}
Index: clang/test/SemaHLSL/shader_type_attr.hlsl
===
--- clang/test/SemaHLSL/shader_type_attr.hlsl
+++ clang/test/SemaHLSL/shader_type_attr.hlsl
@@ -28,7 +28,7 @@
 } // namespace spec
 
 // expected-error@+1 {{'shader' attribute parameters do not match the previous declaration}}
-[shader("compute")]
+[shader("pixel")]
 // expected-note@+1 {{conflicting attribute is here}}
 [shader("vertex")]
 int doubledUp() {
@@ -40,7 +40,7 @@
 int forwardDecl();
 
 // expected-error@+1 {{'shader' attribute parameters do not match the previous declaration}}
-[shader("compute")]
+[shader("compute"), numthreads(8,1,1)]
 int forwardDecl() {
   return 1;
 }
@@ -58,17 +58,17 @@
 #endif // END of FAIL
 
 // CHECK:HLSLShaderAttr 0x{{[0-9a-fA-F]+}}  Compute
-[shader("compute")]
+[shader("compute"), numthreads(8,1,1)]
 int entry() {
   return 1;
 }
 
 // Because these two attributes match, they should both appear in the AST
-[shader("compute")]
+[shader("compute"), numthreads(8,1,1)]
 // CHECK:HLSLShaderAttr 0x{{[0-9a-fA-F]+}}  Compute
 int secondFn();
 
-[shader("compute")]
+[shader("compute"), numthreads(8,1,1)]
 // CHECK:HLSLShaderAttr 0x{{[0-9a-fA-F]+}}  Compute
 int secondFn() {
   return 1;
Index: clang/test/SemaHLSL/entry_shader.hlsl
===
--- clang/test/SemaHLSL/entry_shader.hlsl
+++ clang/test/SemaHLSL/entry_shader.hlsl
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-compute -x hlsl -hlsl-entry foo  -o - %s -DSHADER='"anyHit"' -verify
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-compute -x hlsl -hlsl-entry foo  -o - %s -DSHADER='"mesh"' -verify
 // RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-compute -x hlsl -hlsl-entry foo  -o - %s -DSHADER='"compute"'
 
 // expected-error@+1 {{'shader' attribute on entry function does not match the target profile}}
Index: clang/include/clang/Basic/Attr.td
===
--- clang/include/clang/Basic/Attr.td
+++ clang/include/clang/Basic/Attr.td
@@ -4143,8 +4143,8 @@
 EnumArgument<"Type", "ShaderType",
  [
"pixel", "vertex", "geometry", "hull", "domain", "compute",
-   "library", "raygeneration", "intersection", "anyHit",
-   "closestHit", "miss", "callable", "mesh", "amplification"
+   "library", "raygeneration", "intersection", "anyhit",
+   "closesthit", "miss", "callable", "mesh", "amplification"
  ],
  [
"Pixel", "Vertex", "Geometry", "Hull", "Domain", "Compute",
@@ -4232,4 +4232,3 @@
   let Subjects = SubjectList<[TypedefName], ErrorDiag>;
   let Documentation = [Undocumented];
 }
-
___
cfe-commits mailing list

[PATCH] D158566: Add CLANGD_INCLUDE_TESTS as a separate flag to control clangd tests

[Justin Bogner via Phabricator via cfe-commits]

bogner accepted this revision.
bogner added a comment.
This revision is now accepted and ready to land.

I'm not entirely convinced by the motivation (wouldn't it be better to just fix 
the tests?), but this seems like a perfectly reasonable flag to have.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158566/new/

https://reviews.llvm.org/D158566

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158803: [Sema][HLSL] Consolidate handling of HLSL attributes

[Justin Bogner via Phabricator via cfe-commits]

bogner created this revision.
bogner added reviewers: beanz, python3kgae, bob80905, tex3d.
Herald added subscribers: Anastasia, arphaman, mcrosier.
Herald added a reviewer: aaron.ballman.
Herald added a project: All.
bogner requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This moves the sema checking of the entrypoint sensitive HLSL
attributes all into one place. This ended up being kind of large for a
couple of reasons:

- I had to move the call to CheckHLSLEntryPoint later in 
ActOnFunctionDeclarator so that we do this after redeclarations and have access 
to all of the attributes.

- We need to transfer the target shader stage onto the specified entry point 
before doing the checking.

- I removed "library" from the HLSLShader attribute value enum and just go 
through a string to convert from the triple - the other way was confusing and 
brittle.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D158803

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/CodeGenHLSL/GlobalDestructors.hlsl
  clang/test/SemaHLSL/Semantics/entry_parameter.hlsl
  clang/test/SemaHLSL/Semantics/groupindex.hlsl
  clang/test/SemaHLSL/entry.hlsl
  clang/test/SemaHLSL/entry_shader.hlsl
  clang/test/SemaHLSL/entry_shader_redecl.hlsl
  clang/test/SemaHLSL/num_threads.hlsl
  clang/test/SemaHLSL/shader_type_attr.hlsl

Index: clang/test/SemaHLSL/shader_type_attr.hlsl
===
--- clang/test/SemaHLSL/shader_type_attr.hlsl
+++ clang/test/SemaHLSL/shader_type_attr.hlsl
@@ -28,7 +28,7 @@
 } // namespace spec
 
 // expected-error@+1 {{'shader' attribute parameters do not match the previous declaration}}
-[shader("compute")]
+[shader("pixel")]
 // expected-note@+1 {{conflicting attribute is here}}
 [shader("vertex")]
 int doubledUp() {
@@ -40,7 +40,7 @@
 int forwardDecl();
 
 // expected-error@+1 {{'shader' attribute parameters do not match the previous declaration}}
-[shader("compute")]
+[shader("compute"), numthreads(8,1,1)]
 int forwardDecl() {
   return 1;
 }
@@ -58,17 +58,17 @@
 #endif // END of FAIL
 
 // CHECK:HLSLShaderAttr 0x{{[0-9a-fA-F]+}}  Compute
-[shader("compute")]
+[shader("compute"), numthreads(8,1,1)]
 int entry() {
   return 1;
 }
 
 // Because these two attributes match, they should both appear in the AST
-[shader("compute")]
+[shader("compute"), numthreads(8,1,1)]
 // CHECK:HLSLShaderAttr 0x{{[0-9a-fA-F]+}}  Compute
 int secondFn();
 
-[shader("compute")]
+[shader("compute"), numthreads(8,1,1)]
 // CHECK:HLSLShaderAttr 0x{{[0-9a-fAl-F]+}}  Compute
 int secondFn() {
   return 1;
Index: clang/test/SemaHLSL/num_threads.hlsl
===
--- clang/test/SemaHLSL/num_threads.hlsl
+++ clang/test/SemaHLSL/num_threads.hlsl
@@ -1,7 +1,7 @@
-// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-compute -x hlsl -ast-dump -o - %s | FileCheck %s 
-// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-mesh -x hlsl -ast-dump -o - %s | FileCheck %s 
-// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-amplification -x hlsl -ast-dump -o - %s | FileCheck %s 
-// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-library -x hlsl -ast-dump -o - %s | FileCheck %s 
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-compute -x hlsl -ast-dump -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-mesh -x hlsl -ast-dump -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-amplification -x hlsl -ast-dump -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-library -x hlsl -ast-dump -o - %s | FileCheck %s
 // RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-pixel -x hlsl -ast-dump -o - %s -verify
 // RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-vertex -x hlsl -ast-dump -o - %s -verify
 // RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-hull -x hlsl -ast-dump -o - %s -verify
@@ -97,14 +97,20 @@
   return 1;
 }
 
+[numthreads(4,2,1)]
+// CHECK: HLSLNumThreadsAttr 0x{{[0-9a-fA-F]+}}  4 2 1
+int onlyOnForwardDecl();
+
+// CHECK: HLSLNumThreadsAttr 0x{{[0-9a-fA-F]+}}  Inherited 4 2 1
+int onlyOnForwardDecl() {
+  return 1;
+}
 
 #else // Vertex and Pixel only beyond here
-// expected-error-re@+1 {{attribute 'numthreads' is unsupported in {{[A-Za-z]+}} shaders, requires Compute, Amplification, Mesh or Library}}
+// expected-error-re@+1 {{attribute 'numthreads' is unsupported in {{[A-Za-z]+}} shaders, requires compute, amplification, or mesh}}
 [numthreads(1,1,1)]
 int main() {
  return 1;
 }
 
 #endif
-
-
Index: clang/test/SemaHLSL/entry_shader_redecl.hlsl
===
--- /dev/null
+++ clang/test/SemaHLSL/entry_shader_redecl.hlsl
@@ -0,0 +1,75 @@
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-compute -x hlsl 

[PATCH] D158113: [Driver][DXC] Accept debug flags (/Zi and /Qembed_debug)

[Justin Bogner via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGbe5c8dea1e1d: [Driver][DXC] Accept debug flags (/Zi and 
/Qembed_debug) (authored by bogner).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158113/new/

https://reviews.llvm.org/D158113

Files:
  clang/include/clang/Driver/Options.td
  clang/test/Driver/dxc_debug.hlsl


Index: clang/test/Driver/dxc_debug.hlsl
===
--- /dev/null
+++ clang/test/Driver/dxc_debug.hlsl
@@ -0,0 +1,14 @@
+// RUN: %clang_dxc -Tlib_6_7 -### -g %s 2>&1 | FileCheck %s
+// RUN: %clang_dxc -Tlib_6_7 -### /Zi %s 2>&1 | FileCheck %s
+// RUN: %clang_dxc -Tlib_6_7 -### /Zi /Qembed_debug %s 2>&1 | FileCheck %s
+// RUN: %clang_dxc -Tlib_6_7 -### -Zi %s 2>&1 | FileCheck %s
+// RUN: %clang_dxc -Tlib_6_7 -### -Zi -Qembed_debug %s 2>&1 | FileCheck %s
+// RUN: %clang_dxc -Tlib_6_7 -### -Zi -gcodeview %s 2>&1 | FileCheck %s 
-check-prefixes=CHECK,CHECK-CV
+// RUN: %clang_dxc -Tlib_6_7 -### -Zi -gdwarf %s 2>&1 | FileCheck %s 
-check-prefixes=CHECK,CHECK-DWARF
+// RUN: %clang_dxc -Tlib_6_7 -### -gcodeview -Zi %s 2>&1 | FileCheck %s 
-check-prefixes=CHECK,CHECK-CV
+// RUN: %clang_dxc -Tlib_6_7 -### -gdwarf -Zi %s 2>&1 | FileCheck %s 
-check-prefixes=CHECK,CHECK-DWARF
+
+// CHECK: "-cc1"
+// CHECK-CV-SAME: -gcodeview
+// CHECK-SAME: "-debug-info-kind=constructor"
+// CHECK-DWARF-SAME: -dwarf-version
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -7869,7 +7869,7 @@
 def _SLASH_ZH_SHA_256 : CLFlag<"ZH:SHA_256">,
   HelpText<"Use SHA256 for file checksums in debug info">,
   Alias, AliasArgs<["sha256"]>;
-def _SLASH_Zi : CLFlag<"Zi">, Alias,
+def _SLASH_Zi : CLFlag<"Zi", [CLOption, DXCOption]>, Alias,
   HelpText<"Like /Z7">;
 def _SLASH_Zp : CLJoined<"Zp">,
   HelpText<"Set default maximum struct packing alignment">,
@@ -8202,3 +8202,6 @@
   HelpText<"DXIL validator installation path">;
 def dxc_disable_validation : DXCFlag<"Vd">,
   HelpText<"Disable validation">;
+def : Option<["/", "-"], "Qembed_debug", KIND_FLAG>, Group,
+  Flags<[Ignored, NoXarchOption]>, Visibility<[DXCOption]>,
+  HelpText<"Embed PDB in shader container (ignored)">;


Index: clang/test/Driver/dxc_debug.hlsl
===
--- /dev/null
+++ clang/test/Driver/dxc_debug.hlsl
@@ -0,0 +1,14 @@
+// RUN: %clang_dxc -Tlib_6_7 -### -g %s 2>&1 | FileCheck %s
+// RUN: %clang_dxc -Tlib_6_7 -### /Zi %s 2>&1 | FileCheck %s
+// RUN: %clang_dxc -Tlib_6_7 -### /Zi /Qembed_debug %s 2>&1 | FileCheck %s
+// RUN: %clang_dxc -Tlib_6_7 -### -Zi %s 2>&1 | FileCheck %s
+// RUN: %clang_dxc -Tlib_6_7 -### -Zi -Qembed_debug %s 2>&1 | FileCheck %s
+// RUN: %clang_dxc -Tlib_6_7 -### -Zi -gcodeview %s 2>&1 | FileCheck %s -check-prefixes=CHECK,CHECK-CV
+// RUN: %clang_dxc -Tlib_6_7 -### -Zi -gdwarf %s 2>&1 | FileCheck %s -check-prefixes=CHECK,CHECK-DWARF
+// RUN: %clang_dxc -Tlib_6_7 -### -gcodeview -Zi %s 2>&1 | FileCheck %s -check-prefixes=CHECK,CHECK-CV
+// RUN: %clang_dxc -Tlib_6_7 -### -gdwarf -Zi %s 2>&1 | FileCheck %s -check-prefixes=CHECK,CHECK-DWARF
+
+// CHECK: "-cc1"
+// CHECK-CV-SAME: -gcodeview
+// CHECK-SAME: "-debug-info-kind=constructor"
+// CHECK-DWARF-SAME: -dwarf-version
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -7869,7 +7869,7 @@
 def _SLASH_ZH_SHA_256 : CLFlag<"ZH:SHA_256">,
   HelpText<"Use SHA256 for file checksums in debug info">,
   Alias, AliasArgs<["sha256"]>;
-def _SLASH_Zi : CLFlag<"Zi">, Alias,
+def _SLASH_Zi : CLFlag<"Zi", [CLOption, DXCOption]>, Alias,
   HelpText<"Like /Z7">;
 def _SLASH_Zp : CLJoined<"Zp">,
   HelpText<"Set default maximum struct packing alignment">,
@@ -8202,3 +8202,6 @@
   HelpText<"DXIL validator installation path">;
 def dxc_disable_validation : DXCFlag<"Vd">,
   HelpText<"Disable validation">;
+def : Option<["/", "-"], "Qembed_debug", KIND_FLAG>, Group,
+  Flags<[Ignored, NoXarchOption]>, Visibility<[DXCOption]>,
+  HelpText<"Embed PDB in shader container (ignored)">;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158157: [clang-tidy] Disable implicit search for a compilation db in some tests

[Justin Bogner via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG12babb08189c: [clang-tidy] Disable implicit search for a 
compilation db in some tests (authored by bogner).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158157/new/

https://reviews.llvm.org/D158157

Files:
  clang-tools-extra/test/clang-tidy/checkers/cert/mem57-cpp-cpp17.cpp
  
clang-tools-extra/test/clang-tidy/checkers/readability/else-after-return-pp-no-crash.cpp
  
clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-case-violation.cpp
  
clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-multiple-TUs.cpp
  clang-tools-extra/test/clang-tidy/infrastructure/use-color.cpp


Index: clang-tools-extra/test/clang-tidy/infrastructure/use-color.cpp
===
--- clang-tools-extra/test/clang-tidy/infrastructure/use-color.cpp
+++ clang-tools-extra/test/clang-tidy/infrastructure/use-color.cpp
@@ -5,9 +5,9 @@
 // RUN: clang-tidy -config='UseColor: false' -dump-config | FileCheck 
-check-prefix=CHECK-CONFIG-NO-COLOR %s
 // RUN: clang-tidy -help | FileCheck -check-prefix=CHECK-OPT-PRESENT %s
 
-// RUN: clang-tidy -checks='-*, modernize-use-override' -extra-arg=-std=c++11 
-use-color=false %s | FileCheck -check-prefix=CHECK-NO-COLOR %s
-// RUN: clang-tidy -checks='-*, modernize-use-override' -extra-arg=-std=c++11 
%s | FileCheck -check-prefix=CHECK-NO-COLOR %s
-// RUN: clang-tidy -checks='-*, modernize-use-override' -extra-arg=-std=c++11 
-use-color %s | FileCheck -check-prefix=CHECK-COLOR %s
+// RUN: clang-tidy -checks='-*, modernize-use-override' -use-color=false %s -- 
-std=c++11 | FileCheck -check-prefix=CHECK-NO-COLOR %s
+// RUN: clang-tidy -checks='-*, modernize-use-override' %s -- -std=c++11 | 
FileCheck -check-prefix=CHECK-NO-COLOR %s
+// RUN: clang-tidy -checks='-*, modernize-use-override' -use-color %s -- 
-std=c++11 | FileCheck -check-prefix=CHECK-COLOR %s
 
 // CHECK-NOT: UseColor
 // CHECK-CONFIG-NO-COLOR: UseColor: false
Index: 
clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-multiple-TUs.cpp
===
--- 
clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-multiple-TUs.cpp
+++ 
clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-multiple-TUs.cpp
@@ -1,4 +1,4 @@
-// RUN: clang-tidy %S/Inputs/nolintbeginend/1st-translation-unit.cpp 
%S/Inputs/nolintbeginend/2nd-translation-unit.cpp 
--checks='-*,google-explicit-constructor' 2>&1 | FileCheck %s
+// RUN: clang-tidy %S/Inputs/nolintbeginend/1st-translation-unit.cpp 
%S/Inputs/nolintbeginend/2nd-translation-unit.cpp 
--checks='-*,google-explicit-constructor' -- 2>&1 | FileCheck %s
 
 // CHECK-NOT: 1st-translation-unit.cpp:2:11: warning: single-argument 
constructors must be marked explicit
 // CHECK: 1st-translation-unit.cpp:5:11: warning: single-argument constructors 
must be marked explicit
Index: 
clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-case-violation.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-case-violation.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-case-violation.cpp
@@ -5,7 +5,7 @@
 // RUN:   readability-identifier-naming.ClassCase: UUPER_CASE, \
 // RUN:   readability-identifier-naming.StructCase: CAMEL, \
 // RUN:   readability-identifier-naming.EnumCase: AnY_cASe, \
-// RUN:   }}" 2>&1 | FileCheck %s --implicit-check-not="{{warning|error}}:"
+// RUN:   }}" -- 2>&1 | FileCheck %s --implicit-check-not="{{warning|error}}:"
 
 // CHECK-DAG: warning: invalid configuration value 'camelback' for option 
'readability-identifier-naming.FunctionCase'; did you mean 'camelBack'? 
[clang-tidy-config]
 // CHECK-DAG: warning: invalid configuration value 'UUPER_CASE' for option 
'readability-identifier-naming.ClassCase'; did you mean 'UPPER_CASE'? 
[clang-tidy-config]
Index: 
clang-tools-extra/test/clang-tidy/checkers/readability/else-after-return-pp-no-crash.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/readability/else-after-return-pp-no-crash.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/readability/else-after-return-pp-no-crash.cpp
@@ -1,4 +1,4 @@
-// RUN: clang-tidy %s -checks=-*,readability-else-after-return
+// RUN: clang-tidy %s -checks=-*,readability-else-after-return --
 
 // We aren't concerned about the output here, just want to ensure clang-tidy 
doesn't crash.
 void foo() {
Index: clang-tools-extra/test/clang-tidy/checkers/cert/mem57-cpp-cpp17.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/cert/mem57-cpp-cpp17.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/cert/mem57-cpp-cpp17.cpp
@@ -1,6 +1,6 @@

[PATCH] D158157: [clang-tidy] Disable implicit search for a compilation db in some tests

[Justin Bogner via Phabricator via cfe-commits]

bogner created this revision.
bogner added reviewers: njames93, PiotrZSL, carlosgalvezp.
Herald added subscribers: xazax.hun, mcrosier.
Herald added a project: All.
bogner requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

These tests were failing for me on windows with a very curious error:

  error: argument unused during compilation: '/Zc:preprocessor'

It turns out that they were walking up the directory structure and
finding the compilation DB in my top level llvm-project directory.

Add `--` to the ends of the clang-tidy command lines so that they
don't go looking for random compilation databases. Also replace args
specified with `-extra-arg` with directly specifying them to the
FixedCompilationDatabase.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D158157

Files:
  clang-tools-extra/test/clang-tidy/checkers/cert/mem57-cpp-cpp17.cpp
  
clang-tools-extra/test/clang-tidy/checkers/readability/else-after-return-pp-no-crash.cpp
  
clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-case-violation.cpp
  
clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-multiple-TUs.cpp
  clang-tools-extra/test/clang-tidy/infrastructure/use-color.cpp


Index: clang-tools-extra/test/clang-tidy/infrastructure/use-color.cpp
===
--- clang-tools-extra/test/clang-tidy/infrastructure/use-color.cpp
+++ clang-tools-extra/test/clang-tidy/infrastructure/use-color.cpp
@@ -5,9 +5,9 @@
 // RUN: clang-tidy -config='UseColor: false' -dump-config | FileCheck 
-check-prefix=CHECK-CONFIG-NO-COLOR %s
 // RUN: clang-tidy -help | FileCheck -check-prefix=CHECK-OPT-PRESENT %s
 
-// RUN: clang-tidy -checks='-*, modernize-use-override' -extra-arg=-std=c++11 
-use-color=false %s | FileCheck -check-prefix=CHECK-NO-COLOR %s
-// RUN: clang-tidy -checks='-*, modernize-use-override' -extra-arg=-std=c++11 
%s | FileCheck -check-prefix=CHECK-NO-COLOR %s
-// RUN: clang-tidy -checks='-*, modernize-use-override' -extra-arg=-std=c++11 
-use-color %s | FileCheck -check-prefix=CHECK-COLOR %s
+// RUN: clang-tidy -checks='-*, modernize-use-override' -use-color=false %s -- 
-std=c++11 | FileCheck -check-prefix=CHECK-NO-COLOR %s
+// RUN: clang-tidy -checks='-*, modernize-use-override' %s -- -std=c++11 | 
FileCheck -check-prefix=CHECK-NO-COLOR %s
+// RUN: clang-tidy -checks='-*, modernize-use-override' -use-color %s -- 
-std=c++11 | FileCheck -check-prefix=CHECK-COLOR %s
 
 // CHECK-NOT: UseColor
 // CHECK-CONFIG-NO-COLOR: UseColor: false
Index: 
clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-multiple-TUs.cpp
===
--- 
clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-multiple-TUs.cpp
+++ 
clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-multiple-TUs.cpp
@@ -1,4 +1,4 @@
-// RUN: clang-tidy %S/Inputs/nolintbeginend/1st-translation-unit.cpp 
%S/Inputs/nolintbeginend/2nd-translation-unit.cpp 
--checks='-*,google-explicit-constructor' 2>&1 | FileCheck %s
+// RUN: clang-tidy %S/Inputs/nolintbeginend/1st-translation-unit.cpp 
%S/Inputs/nolintbeginend/2nd-translation-unit.cpp 
--checks='-*,google-explicit-constructor' -- 2>&1 | FileCheck %s
 
 // CHECK-NOT: 1st-translation-unit.cpp:2:11: warning: single-argument 
constructors must be marked explicit
 // CHECK: 1st-translation-unit.cpp:5:11: warning: single-argument constructors 
must be marked explicit
Index: 
clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-case-violation.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-case-violation.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-case-violation.cpp
@@ -5,7 +5,7 @@
 // RUN:   readability-identifier-naming.ClassCase: UUPER_CASE, \
 // RUN:   readability-identifier-naming.StructCase: CAMEL, \
 // RUN:   readability-identifier-naming.EnumCase: AnY_cASe, \
-// RUN:   }}" 2>&1 | FileCheck %s --implicit-check-not="{{warning|error}}:"
+// RUN:   }}" -- 2>&1 | FileCheck %s --implicit-check-not="{{warning|error}}:"
 
 // CHECK-DAG: warning: invalid configuration value 'camelback' for option 
'readability-identifier-naming.FunctionCase'; did you mean 'camelBack'? 
[clang-tidy-config]
 // CHECK-DAG: warning: invalid configuration value 'UUPER_CASE' for option 
'readability-identifier-naming.ClassCase'; did you mean 'UPPER_CASE'? 
[clang-tidy-config]
Index: 
clang-tools-extra/test/clang-tidy/checkers/readability/else-after-return-pp-no-crash.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/readability/else-after-return-pp-no-crash.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/readability/else-after-return-pp-no-crash.cpp
@@ -1,4 +1,4 @@
-// RUN: clang-tidy %s 

[PATCH] D158113: [Driver][DXC] Accept debug flags (/Zi and /Qembed_debug)

[Justin Bogner via Phabricator via cfe-commits]

bogner created this revision.
bogner added a reviewer: beanz.
Herald added a subscriber: mcrosier.
Herald added a project: All.
bogner requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This adds /Zi handling and explicitly ignores /Qembed_debug, which
should make our command line handling compatible enough with dxc that
we can share logic on compiler explorer.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D158113

Files:
  clang/include/clang/Driver/Options.td
  clang/test/Driver/dxc_debug.hlsl


Index: clang/test/Driver/dxc_debug.hlsl
===
--- /dev/null
+++ clang/test/Driver/dxc_debug.hlsl
@@ -0,0 +1,14 @@
+// RUN: %clang_dxc -Tlib_6_7 -### -g %s 2>&1 | FileCheck %s
+// RUN: %clang_dxc -Tlib_6_7 -### /Zi %s 2>&1 | FileCheck %s
+// RUN: %clang_dxc -Tlib_6_7 -### /Zi /Qembed_debug %s 2>&1 | FileCheck %s
+// RUN: %clang_dxc -Tlib_6_7 -### -Zi %s 2>&1 | FileCheck %s
+// RUN: %clang_dxc -Tlib_6_7 -### -Zi -Qembed_debug %s 2>&1 | FileCheck %s
+// RUN: %clang_dxc -Tlib_6_7 -### -Zi -gcodeview %s 2>&1 | FileCheck %s 
-check-prefixes=CHECK,CHECK-CV
+// RUN: %clang_dxc -Tlib_6_7 -### -Zi -gdwarf %s 2>&1 | FileCheck %s 
-check-prefixes=CHECK,CHECK-DWARF
+// RUN: %clang_dxc -Tlib_6_7 -### -gcodeview -Zi %s 2>&1 | FileCheck %s 
-check-prefixes=CHECK,CHECK-CV
+// RUN: %clang_dxc -Tlib_6_7 -### -gdwarf -Zi %s 2>&1 | FileCheck %s 
-check-prefixes=CHECK,CHECK-DWARF
+
+// CHECK: "-cc1"
+// CHECK-CV-SAME: -gcodeview
+// CHECK-SAME: "-debug-info-kind=constructor"
+// CHECK-DWARF-SAME: -dwarf-version
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -7850,7 +7850,7 @@
 def _SLASH_ZH_SHA_256 : CLFlag<"ZH:SHA_256">,
   HelpText<"Use SHA256 for file checksums in debug info">,
   Alias, AliasArgs<["sha256"]>;
-def _SLASH_Zi : CLFlag<"Zi">, Alias,
+def _SLASH_Zi : CLFlag<"Zi", [CLOption, DXCOption]>, Alias,
   HelpText<"Like /Z7">;
 def _SLASH_Zp : CLJoined<"Zp">,
   HelpText<"Set default maximum struct packing alignment">,
@@ -8183,3 +8183,6 @@
   HelpText<"DXIL validator installation path">;
 def dxc_disable_validation : DXCFlag<"Vd">,
   HelpText<"Disable validation">;
+def : Option<["/", "-"], "Qembed_debug", KIND_FLAG>, Group,
+  Flags<[Ignored, NoXarchOption]>, Visibility<[DXCOption]>,
+  HelpText<"Embed PDB in shader container (ignored)">;


Index: clang/test/Driver/dxc_debug.hlsl
===
--- /dev/null
+++ clang/test/Driver/dxc_debug.hlsl
@@ -0,0 +1,14 @@
+// RUN: %clang_dxc -Tlib_6_7 -### -g %s 2>&1 | FileCheck %s
+// RUN: %clang_dxc -Tlib_6_7 -### /Zi %s 2>&1 | FileCheck %s
+// RUN: %clang_dxc -Tlib_6_7 -### /Zi /Qembed_debug %s 2>&1 | FileCheck %s
+// RUN: %clang_dxc -Tlib_6_7 -### -Zi %s 2>&1 | FileCheck %s
+// RUN: %clang_dxc -Tlib_6_7 -### -Zi -Qembed_debug %s 2>&1 | FileCheck %s
+// RUN: %clang_dxc -Tlib_6_7 -### -Zi -gcodeview %s 2>&1 | FileCheck %s -check-prefixes=CHECK,CHECK-CV
+// RUN: %clang_dxc -Tlib_6_7 -### -Zi -gdwarf %s 2>&1 | FileCheck %s -check-prefixes=CHECK,CHECK-DWARF
+// RUN: %clang_dxc -Tlib_6_7 -### -gcodeview -Zi %s 2>&1 | FileCheck %s -check-prefixes=CHECK,CHECK-CV
+// RUN: %clang_dxc -Tlib_6_7 -### -gdwarf -Zi %s 2>&1 | FileCheck %s -check-prefixes=CHECK,CHECK-DWARF
+
+// CHECK: "-cc1"
+// CHECK-CV-SAME: -gcodeview
+// CHECK-SAME: "-debug-info-kind=constructor"
+// CHECK-DWARF-SAME: -dwarf-version
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -7850,7 +7850,7 @@
 def _SLASH_ZH_SHA_256 : CLFlag<"ZH:SHA_256">,
   HelpText<"Use SHA256 for file checksums in debug info">,
   Alias, AliasArgs<["sha256"]>;
-def _SLASH_Zi : CLFlag<"Zi">, Alias,
+def _SLASH_Zi : CLFlag<"Zi", [CLOption, DXCOption]>, Alias,
   HelpText<"Like /Z7">;
 def _SLASH_Zp : CLJoined<"Zp">,
   HelpText<"Set default maximum struct packing alignment">,
@@ -8183,3 +8183,6 @@
   HelpText<"DXIL validator installation path">;
 def dxc_disable_validation : DXCFlag<"Vd">,
   HelpText<"Disable validation">;
+def : Option<["/", "-"], "Qembed_debug", KIND_FLAG>, Group,
+  Flags<[Ignored, NoXarchOption]>, Visibility<[DXCOption]>,
+  HelpText<"Embed PDB in shader container (ignored)">;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158037: [Driver][DXC] Remove a bunch of options from DXC

[Justin Bogner via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGe17667b33f1f: [Driver][DXC] Remove a bunch of options from 
DXC (authored by bogner).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158037/new/

https://reviews.llvm.org/D158037

Files:
  clang/include/clang/Driver/Options.td

Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -195,8 +195,7 @@
 def m_wasm_Features_Driver_Group : OptionGroup<"">,
Group, DocName<"WebAssembly Driver">;
 def m_x86_Features_Group : OptionGroup<"">,
-   Group,
-   Visibility<[ClangOption, CLOption, DXCOption]>,
+   Group, Visibility<[ClangOption, CLOption]>,
DocName<"X86">;
 def m_riscv_Features_Group : OptionGroup<"">,
  Group, DocName<"RISC-V">;
@@ -274,8 +273,7 @@
 // Retired with clang-16.0, to provide a deprecation period; it should
 // be removed in Clang 18 or later.
 def enable_trivial_var_init_zero : Flag<["-"], "enable-trivial-auto-var-init-zero-knowing-it-will-be-removed-from-clang">,
-  Flags<[NoArgumentUnused]>,
-  Visibility<[ClangOption, CC1Option, CLOption, DXCOption]>,
+  Flags<[NoArgumentUnused]>, Visibility<[ClangOption, CC1Option, CLOption]>,
   Group;
 
 // Group that ignores all gcc optimizations that won't be implemented
@@ -1079,7 +1077,7 @@
   Visibility<[ClangOption, CLOption, DXCOption]>,
   HelpText<"User directory for configuration files">;
 def coverage : Flag<["-", "--"], "coverage">, Group,
-  Visibility<[ClangOption, CLOption, DXCOption]>;
+  Visibility<[ClangOption, CLOption]>;
 def cpp_precomp : Flag<["-"], "cpp-precomp">, Group;
 def current__version : JoinedOrSeparate<["-"], "current_version">;
 def cxx_isystem : JoinedOrSeparate<["-"], "cxx-isystem">, Group,
@@ -1449,7 +1447,7 @@
   PosFlag,
   NegFlag,
-  BothFlags<[], [ClangOption, CLOption, DXCOption]>>;
+  BothFlags<[], [ClangOption, CLOption]>>;
 
 def fassume_sane_operator_new : Flag<["-"], "fassume-sane-operator-new">, Group;
 def fastcp : Flag<["-"], "fastcp">, Group;
@@ -1488,7 +1486,7 @@
 
 defm experimental_library : BoolFOption<"experimental-library",
   LangOpts<"ExperimentalLibrary">, DefaultFalse,
-  PosFlag>;
 
 def fprofile_sample_use : Flag<["-"], "fprofile-sample-use">, Group,
-Visibility<[ClangOption, CLOption, DXCOption]>;
+Visibility<[ClangOption, CLOption]>;
 def fno_profile_sample_use : Flag<["-"], "fno-profile-sample-use">, Group,
-Visibility<[ClangOption, CLOption, DXCOption]>;
+Visibility<[ClangOption, CLOption]>;
 def fprofile_sample_use_EQ : Joined<["-"], "fprofile-sample-use=">,
 Group, Flags<[NoXarchOption]>,
 Visibility<[ClangOption, CC1Option]>,
@@ -1568,8 +1566,7 @@
 Visibility<[ClangOption, CC1Option, CC1AsOption, CLOption, DXCOption]>,
 Alias;
 def fcoverage_compilation_dir_EQ : Joined<["-"], "fcoverage-compilation-dir=">,
-Group,
-Visibility<[ClangOption, CC1Option, CC1AsOption, CLOption, DXCOption]>,
+Group, Visibility<[ClangOption, CC1Option, CC1AsOption, CLOption]>,
 HelpText<"The compilation directory to embed in the coverage mapping.">,
 MarshallingInfoString>;
 def ffile_compilation_dir_EQ : Joined<["-"], "ffile-compilation-dir=">, Group,
@@ -1581,19 +1578,18 @@
   "Emit extra debug info to make sample profile more accurate">,
   NegFlag>;
 def fprofile_instr_generate : Flag<["-"], "fprofile-instr-generate">,
-Group, Visibility<[ClangOption, CLOption, DXCOption]>,
+Group, Visibility<[ClangOption, CLOption]>,
 HelpText<"Generate instrumented code to collect execution counts into default.profraw file (overridden by '=' form of option or LLVM_PROFILE_FILE env var)">;
 def fprofile_instr_generate_EQ : Joined<["-"], "fprofile-instr-generate=">,
-Group, Visibility<[ClangOption, CLOption, DXCOption]>,
-MetaVarName<"">,
+Group, Visibility<[ClangOption, CLOption]>, MetaVarName<"">,
 HelpText<"Generate instrumented code to collect execution counts into  (overridden by LLVM_PROFILE_FILE env var)">;
 def fprofile_instr_use : Flag<["-"], "fprofile-instr-use">, Group,
-Visibility<[ClangOption, CLOption, DXCOption]>;
+Visibility<[ClangOption, CLOption]>;
 def fprofile_instr_use_EQ : Joined<["-"], "fprofile-instr-use=">,
-Group, Visibility<[ClangOption, CLOption, DXCOption]>,
+Group, Visibility<[ClangOption, CLOption]>,
 HelpText<"Use instrumentation data for profile-guided optimization">;
 def fprofile_remapping_file_EQ : Joined<["-"], "fprofile-remapping-file=">,
-Group, Visibility<[ClangOption, CC1Option, CLOption, DXCOption]>,
+Group, Visibility<[ClangOption, CC1Option, CLOption]>,
 MetaVarName<"">,
 HelpText<"Use the remappings described in  

[PATCH] D157582: [Driver][DXC] Handle -Fo and -Fc flags

[Justin Bogner via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGcb6fe61be3de: [Driver][DXC] Handle -Fo and -Fc flags 
(authored by bogner).

Changed prior to commit:
  https://reviews.llvm.org/D157582?vs=548879=550545#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157582/new/

https://reviews.llvm.org/D157582

Files:
  clang/include/clang/Driver/Options.td
  clang/include/clang/Driver/Types.def
  clang/include/clang/Driver/Types.h
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/HLSL.cpp
  clang/lib/Driver/Types.cpp
  clang/test/Driver/dxc_dxv_path.hlsl
  clang/test/Driver/dxc_output.hlsl
  llvm/lib/MC/MCParser/AsmParser.cpp

Index: llvm/lib/MC/MCParser/AsmParser.cpp
===
--- llvm/lib/MC/MCParser/AsmParser.cpp
+++ llvm/lib/MC/MCParser/AsmParser.cpp
@@ -807,7 +807,7 @@
 PlatformParser.reset(createXCOFFAsmParser());
 break;
   case MCContext::IsDXContainer:
-llvm_unreachable("DXContainer is not supported yet");
+report_fatal_error("DXContainer is not supported yet");
 break;
   }
 
Index: clang/test/Driver/dxc_output.hlsl
===
--- /dev/null
+++ clang/test/Driver/dxc_output.hlsl
@@ -0,0 +1,42 @@
+// With no output args, we emit assembly to stdout.
+// RUN: %clang_dxc -T lib_6_7 -Vd %s -### 2>&1 | FileCheck %s --check-prefixes=CHECK-CC1,CHECK-CC1-STDOUT
+// RUN: %clang_dxc -T lib_6_7 -Vd %s -ccc-print-phases 2>&1 | FileCheck %s --check-prefixes=CHECK-PHASES,CHECK-PHASES-ASM
+
+// Same if we explicitly ask for assembly (-Fc) to stdout.
+// RUN: %clang_dxc -T lib_6_7 -Vd %s -Fc - -### 2>&1 | FileCheck %s --check-prefixes=CHECK-CC1,CHECK-CC1-STDOUT
+// RUN: %clang_dxc -T lib_6_7 -Vd %s -Fc - -ccc-print-phases 2>&1 | FileCheck %s --check-prefixes=CHECK-PHASES,CHECK-PHASES-ASM
+
+// DXIL Assembly to a file.
+// RUN: %clang_dxc -T lib_6_7 -Vd %s -Fc x.asm -### 2>&1 | FileCheck %s --check-prefixes=CHECK-CC1,CHECK-CC1-ASM
+// RUN: %clang_dxc -T lib_6_7 -Vd %s -Fcx.asm -### 2>&1 | FileCheck %s --check-prefixes=CHECK-CC1,CHECK-CC1-ASM
+// RUN: %clang_dxc -T lib_6_7 -Vd %s -Fc x.asm -ccc-print-phases 2>&1 | FileCheck %s --check-prefixes=CHECK-PHASES,CHECK-PHASES-ASM
+
+// DXIL Object code to a file.
+// RUN: %clang_dxc -T lib_6_7 -Vd %s -Fo x.obj -### 2>&1 | FileCheck %s --check-prefixes=CHECK-CC1,CHECK-CC1-OBJ
+// RUN: %clang_dxc -T lib_6_7 -Vd %s -Fox.obj -### 2>&1 | FileCheck %s --check-prefixes=CHECK-CC1,CHECK-CC1-OBJ
+// RUN: %clang_dxc -T lib_6_7 -Vd %s -Fo x.obj -ccc-print-phases 2>&1 | FileCheck %s --check-prefixes=CHECK-PHASES,CHECK-PHASES-OBJ
+
+// If both -Fc and -Fo are provided, we generate both files.
+// RUN: %clang_dxc -T lib_6_7 -Vd %s -Fc x.asm -Fo x.obj -### 2>&1 | FileCheck %s --check-prefixes=CHECK-CC1,CHECK-CC1-ASM,CHECK-CC1-BOTH
+// RUN: %clang_dxc -T lib_6_7 -Vd %s -Fc x.asm -Fo x.obj -ccc-print-phases 2>&1 | FileCheck %s --check-prefixes=CHECK-PHASES,CHECK-PHASES-OBJ
+
+// CHECK-PHASES: 0: input, {{.*}}, hlsl
+// CHECK-PHASES: 1: preprocessor, {0}, c++-cpp-output
+// CHECK-PHASES: 2: compiler, {1}, ir
+// CHECK-PHASES: 3: backend, {2}, assembler
+// CHECK-PHASES-ASM-NOT: 4: assembler, {3}, object
+// CHECK-PHASES-OBJ: 4: assembler, {3}, object
+
+// CHECK-CC1: "-cc1"
+// CHECK-CC1-STDOUT-SAME: "-o" "-"
+
+// CHECK-CC1-ASM-SAME: "-S"
+// CHECK-CC1-ASM-SAME: "-o" "x.asm"
+
+// CHECK-CC1-OBJ-SAME: "-emit-obj"
+// CHECK-CC1-OBJ-SAME: "-o" "x.obj"
+
+// For the case where we specify both -Fc and -Fo, we emit the asm as part of
+// cc1 and invoke cc1as for the object.
+// CHECK-CC1-BOTH: "-cc1as"
+// CHECK-CC1-BOTH-SAME: "-o" "x.obj"
Index: clang/test/Driver/dxc_dxv_path.hlsl
===
--- clang/test/Driver/dxc_dxv_path.hlsl
+++ clang/test/Driver/dxc_dxv_path.hlsl
@@ -12,7 +12,7 @@
 
 // RUN: %clang_dxc -Tlib_6_3 -ccc-print-bindings --dxv-path=%T -Fo %t.dxo  %s 2>&1 | FileCheck %s --check-prefix=BINDINGS
 // BINDINGS: "dxil-unknown-shadermodel6.3-library" - "clang", inputs: ["[[INPUT:.+]]"], output: "[[DXC:.+]].dxo"
-// BINDINGS-NEXT: "dxil-unknown-shadermodel6.3-library" - "hlsl::Validator", inputs: ["[[DXC]].dxo"], output: "[[DXC]].dxo"
+// BINDINGS-NEXT: "dxil-unknown-shadermodel6.3-library" - "hlsl::Validator", inputs: ["[[DXC]].dxo"]
 
 // RUN: %clang_dxc -Tlib_6_3 -ccc-print-phases --dxv-path=%T -Fo %t.dxc  %s 2>&1 | FileCheck %s --check-prefix=PHASES
 
@@ -20,4 +20,5 @@
 // PHASES-NEXT: 1: preprocessor, {0}, c++-cpp-output
 // PHASES-NEXT: 2: compiler, {1}, ir
 // PHASES-NEXT: 3: backend, {2}, assembler
-// PHASES-NEXT: 4: binary-analyzer, {3}, dx-container
+// PHASES-NEXT: 4: assembler, {3}, object
+// PHASES-NEXT: 5: binary-analyzer, {4}, dx-container
Index: clang/lib/Driver/Types.cpp
===
--- 

[PATCH] D158037: [Driver][DXC] Remove a bunch of options from DXC

[Justin Bogner via Phabricator via cfe-commits]

bogner created this revision.
bogner added reviewers: beanz, python3kgae.
Herald added a subscriber: mcrosier.
Herald added a reviewer: sscalpone.
Herald added a project: All.
bogner requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Remove DXCOption from a whole bunch of options that we probably won't
support in the DXC driver. The initial clang-dxc support just made
everything that was a "CoreOption" available, regardless of whether it
made sense. Here I don't remove all of them, but this makes a dent on
making the list a bit more sensible. We can easily add or remove more
if they make sense later.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D158037

Files:
  clang/include/clang/Driver/Options.td

Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -195,8 +195,7 @@
 def m_wasm_Features_Driver_Group : OptionGroup<"">,
Group, DocName<"WebAssembly Driver">;
 def m_x86_Features_Group : OptionGroup<"">,
-   Group,
-   Visibility<[ClangOption, CLOption, DXCOption]>,
+   Group, Visibility<[ClangOption, CLOption]>,
DocName<"X86">;
 def m_riscv_Features_Group : OptionGroup<"">,
  Group, DocName<"RISC-V">;
@@ -274,8 +273,7 @@
 // Retired with clang-16.0, to provide a deprecation period; it should
 // be removed in Clang 18 or later.
 def enable_trivial_var_init_zero : Flag<["-"], "enable-trivial-auto-var-init-zero-knowing-it-will-be-removed-from-clang">,
-  Flags<[NoArgumentUnused]>,
-  Visibility<[ClangOption, CC1Option, CLOption, DXCOption]>,
+  Flags<[NoArgumentUnused]>, Visibility<[ClangOption, CC1Option, CLOption]>,
   Group;
 
 // Group that ignores all gcc optimizations that won't be implemented
@@ -1079,7 +1077,7 @@
   Visibility<[ClangOption, CLOption, DXCOption]>,
   HelpText<"User directory for configuration files">;
 def coverage : Flag<["-", "--"], "coverage">, Group,
-  Visibility<[ClangOption, CLOption, DXCOption]>;
+  Visibility<[ClangOption, CLOption]>;
 def cpp_precomp : Flag<["-"], "cpp-precomp">, Group;
 def current__version : JoinedOrSeparate<["-"], "current_version">;
 def cxx_isystem : JoinedOrSeparate<["-"], "cxx-isystem">, Group,
@@ -1449,7 +1447,7 @@
   PosFlag,
   NegFlag,
-  BothFlags<[], [ClangOption, CLOption, DXCOption]>>;
+  BothFlags<[], [ClangOption, CLOption]>>;
 
 def fassume_sane_operator_new : Flag<["-"], "fassume-sane-operator-new">, Group;
 def fastcp : Flag<["-"], "fastcp">, Group;
@@ -1488,7 +1486,7 @@
 
 defm experimental_library : BoolFOption<"experimental-library",
   LangOpts<"ExperimentalLibrary">, DefaultFalse,
-  PosFlag>;
 
 def fprofile_sample_use : Flag<["-"], "fprofile-sample-use">, Group,
-Visibility<[ClangOption, CLOption, DXCOption]>;
+Visibility<[ClangOption, CLOption]>;
 def fno_profile_sample_use : Flag<["-"], "fno-profile-sample-use">, Group,
-Visibility<[ClangOption, CLOption, DXCOption]>;
+Visibility<[ClangOption, CLOption]>;
 def fprofile_sample_use_EQ : Joined<["-"], "fprofile-sample-use=">,
 Group, Flags<[NoXarchOption]>,
 Visibility<[ClangOption, CC1Option]>,
@@ -1568,8 +1566,7 @@
 Visibility<[ClangOption, CC1Option, CC1AsOption, CLOption, DXCOption]>,
 Alias;
 def fcoverage_compilation_dir_EQ : Joined<["-"], "fcoverage-compilation-dir=">,
-Group,
-Visibility<[ClangOption, CC1Option, CC1AsOption, CLOption, DXCOption]>,
+Group, Visibility<[ClangOption, CC1Option, CC1AsOption, CLOption]>,
 HelpText<"The compilation directory to embed in the coverage mapping.">,
 MarshallingInfoString>;
 def ffile_compilation_dir_EQ : Joined<["-"], "ffile-compilation-dir=">, Group,
@@ -1581,19 +1578,18 @@
   "Emit extra debug info to make sample profile more accurate">,
   NegFlag>;
 def fprofile_instr_generate : Flag<["-"], "fprofile-instr-generate">,
-Group, Visibility<[ClangOption, CLOption, DXCOption]>,
+Group, Visibility<[ClangOption, CLOption]>,
 HelpText<"Generate instrumented code to collect execution counts into default.profraw file (overridden by '=' form of option or LLVM_PROFILE_FILE env var)">;
 def fprofile_instr_generate_EQ : Joined<["-"], "fprofile-instr-generate=">,
-Group, Visibility<[ClangOption, CLOption, DXCOption]>,
-MetaVarName<"">,
+Group, Visibility<[ClangOption, CLOption]>, MetaVarName<"">,
 HelpText<"Generate instrumented code to collect execution counts into  (overridden by LLVM_PROFILE_FILE env var)">;
 def fprofile_instr_use : Flag<["-"], "fprofile-instr-use">, Group,
-Visibility<[ClangOption, CLOption, DXCOption]>;
+Visibility<[ClangOption, CLOption]>;
 def fprofile_instr_use_EQ : Joined<["-"], "fprofile-instr-use=">,
-Group, Visibility<[ClangOption, 

[PATCH] D157150: [Driver] Update BoolOption to handle Visibility. NFC

[Justin Bogner via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG0db9dfb19329: [Driver] Update BoolOption to handle 
Visibility. NFC (authored by bogner).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157150/new/

https://reviews.llvm.org/D157150

Files:
  clang/include/clang/Driver/Options.h
  clang/include/clang/Driver/Options.td

Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -79,6 +79,23 @@
 // target will lead to an err_drv_unsupported_opt_for_target error.
 def TargetSpecific : OptionFlag;
 
+// Indicates that this warning is ignored, but accepted with a warning for
+// GCC compatibility.
+class IgnoredGCCCompat : Flags<[HelpHidden]> {}
+
+class TargetSpecific : Flags<[TargetSpecific]> {}
+
+/
+// Visibility
+
+// We prefer the name "ClangOption" here rather than "Default" to make
+// it clear that these options will be visible in the clang driver (as
+// opposed to clang -cc1, the CL driver, or the flang driver).
+defvar ClangOption = DefaultVis;
+
+/
+// Docs
+
 // A short name to show in documentation. The name will be interpreted as rST.
 class DocName { string DocName = name; }
 
@@ -89,12 +106,6 @@
 // documentation.
 class DocFlatten { bit DocFlatten = 1; }
 
-// Indicates that this warning is ignored, but accepted with a warning for
-// GCC compatibility.
-class IgnoredGCCCompat : Flags<[HelpHidden]> {}
-
-class TargetSpecific : Flags<[TargetSpecific]> {}
-
 /
 // Groups
 
@@ -381,7 +392,8 @@
 
 // Definition of single command line flag. This is an implementation detail, use
 // SetTrueBy or SetFalseBy instead.
-class FlagDef option_flags,
+class FlagDef option_flags, list option_vis,
   string help, list implied_by_expressions = []> {
   // The polarity. Besides spelling, this also decides whether the TableGen
   // record will be prefixed with "no_".
@@ -390,9 +402,12 @@
   // The value assigned to key path when the flag is present on command line.
   bit Value = value;
 
-  // OptionFlags that control visibility of the flag in different tools.
+  // OptionFlags in different tools.
   list OptionFlags = option_flags;
 
+  // OptionVisibility flags for different tools.
+  list OptionVisibility = option_vis;
+
   // The help text associated with the flag.
   string Help = help;
 
@@ -401,8 +416,11 @@
 }
 
 // Additional information to be appended to both positive and negative flag.
-class BothFlags option_flags, string help = ""> {
+class BothFlags option_flags,
+list option_vis = [ClangOption],
+string help = ""> {
   list OptionFlags = option_flags;
+  list OptionVisibility = option_vis;
   string Help = help;
 }
 
@@ -411,23 +429,26 @@
   FlagDef Result
 = FlagDef;
 }
 
 // Definition of the command line flag with positive spelling, e.g. "-ffoo".
-class PosFlag flags = [], string help = "",
-  list implied_by_expressions = []>
-  : FlagDef {}
+class PosFlag flags = [], list vis = [],
+  string help = "", list implied_by_expressions = []>
+  : FlagDef {}
 
 // Definition of the command line flag with negative spelling, e.g. "-fno-foo".
-class NegFlag flags = [], string help = "",
-  list implied_by_expressions = []>
-  : FlagDef {}
+class NegFlag flags = [], list vis = [],
+  string help = "", list implied_by_expressions = []>
+  : FlagDef {}
 
 // Expanded FlagDef that's convenient for creation of TableGen records.
 class FlagDefExpanded
-  : FlagDef {
+  : FlagDef {
   // Name of the TableGen record.
   string RecordName = prefix # !if(flag.Polarity, "", "no_") # name;
 
@@ -445,7 +466,8 @@
 class MarshalledFlagRec
-  : Flag<["-"], flag.Spelling>, Flags, HelpText,
+  : Flag<["-"], flag.Spelling>, Flags,
+Visibility, HelpText,
 MarshallingInfoBooleanFlag,
 ImpliedByAnyOf {}
@@ -459,7 +481,7 @@
 multiclass BoolOption> {
+  BothFlags suffix = BothFlags<[]>> {
   defvar flag1 = FlagDefExpanded.Result, prefix,
  NAME, spelling_base>;
 
@@ -490,7 +512,7 @@
 /// CompilerInvocation.
 multiclass BoolFOption> {
+   BothFlags both = BothFlags<[]>> {
   defm NAME : BoolOption<"f", flag_base, kpm, default, flag1, flag2, both>,
   Group;
 }
@@ -501,7 +523,7 @@
 // CompilerInvocation.
 multiclass BoolGOption> {
+   BothFlags both = BothFlags<[]>> {
   defm NAME : BoolOption<"g", flag_base, kpm, default, flag1, flag2, both>,
   Group;
 }
@@ -509,7 +531,7 @@
 // Works like BoolOption except without marshalling
 multiclass BoolOptionWithoutMarshalling> {
+BothFlags suffix = BothFlags<[]>> {
   defvar flag1 = FlagDefExpanded.Result, prefix,

[PATCH] D157150: [Driver] Update BoolOption to handle Visibility. NFC

[Justin Bogner via Phabricator via cfe-commits]

bogner updated this revision to Diff 550224.
bogner added a comment.

Rebase on top of "Visibility" naming.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157150/new/

https://reviews.llvm.org/D157150

Files:
  clang/include/clang/Driver/Options.h
  clang/include/clang/Driver/Options.td

Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -79,6 +79,23 @@
 // target will lead to an err_drv_unsupported_opt_for_target error.
 def TargetSpecific : OptionFlag;
 
+// Indicates that this warning is ignored, but accepted with a warning for
+// GCC compatibility.
+class IgnoredGCCCompat : Flags<[HelpHidden]> {}
+
+class TargetSpecific : Flags<[TargetSpecific]> {}
+
+/
+// Visibility
+
+// We prefer the name "ClangOption" here rather than "Default" to make
+// it clear that these options will be visible in the clang driver (as
+// opposed to clang -cc1, the CL driver, or the flang driver).
+defvar ClangOption = DefaultVis;
+
+/
+// Docs
+
 // A short name to show in documentation. The name will be interpreted as rST.
 class DocName { string DocName = name; }
 
@@ -89,12 +106,6 @@
 // documentation.
 class DocFlatten { bit DocFlatten = 1; }
 
-// Indicates that this warning is ignored, but accepted with a warning for
-// GCC compatibility.
-class IgnoredGCCCompat : Flags<[HelpHidden]> {}
-
-class TargetSpecific : Flags<[TargetSpecific]> {}
-
 /
 // Groups
 
@@ -381,7 +392,8 @@
 
 // Definition of single command line flag. This is an implementation detail, use
 // SetTrueBy or SetFalseBy instead.
-class FlagDef option_flags,
+class FlagDef option_flags, list option_vis,
   string help, list implied_by_expressions = []> {
   // The polarity. Besides spelling, this also decides whether the TableGen
   // record will be prefixed with "no_".
@@ -390,9 +402,12 @@
   // The value assigned to key path when the flag is present on command line.
   bit Value = value;
 
-  // OptionFlags that control visibility of the flag in different tools.
+  // OptionFlags in different tools.
   list OptionFlags = option_flags;
 
+  // OptionVisibility flags for different tools.
+  list OptionVisibility = option_vis;
+
   // The help text associated with the flag.
   string Help = help;
 
@@ -401,8 +416,11 @@
 }
 
 // Additional information to be appended to both positive and negative flag.
-class BothFlags option_flags, string help = ""> {
+class BothFlags option_flags,
+list option_vis = [ClangOption],
+string help = ""> {
   list OptionFlags = option_flags;
+  list OptionVisibility = option_vis;
   string Help = help;
 }
 
@@ -411,23 +429,26 @@
   FlagDef Result
 = FlagDef;
 }
 
 // Definition of the command line flag with positive spelling, e.g. "-ffoo".
-class PosFlag flags = [], string help = "",
-  list implied_by_expressions = []>
-  : FlagDef {}
+class PosFlag flags = [], list vis = [],
+  string help = "", list implied_by_expressions = []>
+  : FlagDef {}
 
 // Definition of the command line flag with negative spelling, e.g. "-fno-foo".
-class NegFlag flags = [], string help = "",
-  list implied_by_expressions = []>
-  : FlagDef {}
+class NegFlag flags = [], list vis = [],
+  string help = "", list implied_by_expressions = []>
+  : FlagDef {}
 
 // Expanded FlagDef that's convenient for creation of TableGen records.
 class FlagDefExpanded
-  : FlagDef {
+  : FlagDef {
   // Name of the TableGen record.
   string RecordName = prefix # !if(flag.Polarity, "", "no_") # name;
 
@@ -445,7 +466,8 @@
 class MarshalledFlagRec
-  : Flag<["-"], flag.Spelling>, Flags, HelpText,
+  : Flag<["-"], flag.Spelling>, Flags,
+Visibility, HelpText,
 MarshallingInfoBooleanFlag,
 ImpliedByAnyOf {}
@@ -459,7 +481,7 @@
 multiclass BoolOption> {
+  BothFlags suffix = BothFlags<[]>> {
   defvar flag1 = FlagDefExpanded.Result, prefix,
  NAME, spelling_base>;
 
@@ -490,7 +512,7 @@
 /// CompilerInvocation.
 multiclass BoolFOption> {
+   BothFlags both = BothFlags<[]>> {
   defm NAME : BoolOption<"f", flag_base, kpm, default, flag1, flag2, both>,
   Group;
 }
@@ -501,7 +523,7 @@
 // CompilerInvocation.
 multiclass BoolGOption> {
+   BothFlags both = BothFlags<[]>> {
   defm NAME : BoolOption<"g", flag_base, kpm, default, flag1, flag2, both>,
   Group;
 }
@@ -509,7 +531,7 @@
 // Works like BoolOption except without marshalling
 multiclass BoolOptionWithoutMarshalling> {
+BothFlags suffix = BothFlags<[]>> {
   defvar flag1 = FlagDefExpanded.Result, prefix,
  NAME, spelling_base>;
 
@@ -531,10 +553,12 @@
   defvar implied = !if(flag1.CanBeImplied, flag1, 

[PATCH] D157151: [Driver] Refactor to use llvm Option's new Visibility flags

[Justin Bogner via Phabricator via cfe-commits]

bogner added a comment.

In D157151#4587522 , @awarzynski 
wrote:

> This might cause some disruption to downstream consumers of this API and 
> Options.td. Hopefully, "update_options_td_flags.py" that you've included 
> should minimise that. I suggest "advertising" it in the summary a bit more.
>
> LGTM, great work, thank you!
>
> When landing this, could you send a PSA to Discourse to make folks aware of 
> what's coming? I am primarily concerned about our downstream users.

Good call, and that should help a bit with advertising what to do if a 
downstream breaks. I'll do so




Comment at: llvm/unittests/Option/OptionParsingTest.cpp:18
 
+#pragma clang diagnostic ignored "-Wdeprecated-declarations"
+

awarzynski wrote:
> Why not just update the test?
I'd rather not remove the tests for the old API until we actually remove the API


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157151/new/

https://reviews.llvm.org/D157151

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D157149: [Option] Add "Visibility" field and clone the OptTable APIs to use it

[Justin Bogner via Phabricator via cfe-commits]

bogner added inline comments.



Comment at: llvm/include/llvm/Option/OptParser.td:153
 class Flags flags> { list Flags = flags; }
+class Vis vis> { list Vis = vis; }
 class Group { OptionGroup Group = group; }

phosek wrote:
> Would it be possible to spell this out in full, that is `Visibility` rather 
> than `Vis`? It's more consistent with all other attributes and improves 
> readability in my opinion.
Sure, I can change it. I went with brevity thinking that it would help 
readability, especially in clang's Options.td, but I'm fairly okay with either 
way.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157149/new/

https://reviews.llvm.org/D157149

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D157150: [Driver] Update BoolOption to handle Visibility. NFC

[Justin Bogner via Phabricator via cfe-commits]

bogner updated this revision to Diff 550125.
bogner edited the summary of this revision.
bogner added a comment.

Rebase on top of "DefaultVis" naming


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157150/new/

https://reviews.llvm.org/D157150

Files:
  clang/include/clang/Driver/Options.h
  clang/include/clang/Driver/Options.td

Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -79,6 +79,23 @@
 // target will lead to an err_drv_unsupported_opt_for_target error.
 def TargetSpecific : OptionFlag;
 
+// Indicates that this warning is ignored, but accepted with a warning for
+// GCC compatibility.
+class IgnoredGCCCompat : Flags<[HelpHidden]> {}
+
+class TargetSpecific : Flags<[TargetSpecific]> {}
+
+/
+// Visibility
+
+// We prefer the name "ClangOption" here rather than "Default" to make
+// it clear that these options will be visible in the clang driver (as
+// opposed to clang -cc1, the CL driver, or the flang driver).
+defvar ClangOption = DefaultVis;
+
+/
+// Docs
+
 // A short name to show in documentation. The name will be interpreted as rST.
 class DocName { string DocName = name; }
 
@@ -89,12 +106,6 @@
 // documentation.
 class DocFlatten { bit DocFlatten = 1; }
 
-// Indicates that this warning is ignored, but accepted with a warning for
-// GCC compatibility.
-class IgnoredGCCCompat : Flags<[HelpHidden]> {}
-
-class TargetSpecific : Flags<[TargetSpecific]> {}
-
 /
 // Groups
 
@@ -381,7 +392,8 @@
 
 // Definition of single command line flag. This is an implementation detail, use
 // SetTrueBy or SetFalseBy instead.
-class FlagDef option_flags,
+class FlagDef option_flags, list option_vis,
   string help, list implied_by_expressions = []> {
   // The polarity. Besides spelling, this also decides whether the TableGen
   // record will be prefixed with "no_".
@@ -390,9 +402,12 @@
   // The value assigned to key path when the flag is present on command line.
   bit Value = value;
 
-  // OptionFlags that control visibility of the flag in different tools.
+  // OptionFlags in different tools.
   list OptionFlags = option_flags;
 
+  // OptionVisibility flags for different tools.
+  list OptionVis = option_vis;
+
   // The help text associated with the flag.
   string Help = help;
 
@@ -401,8 +416,11 @@
 }
 
 // Additional information to be appended to both positive and negative flag.
-class BothFlags option_flags, string help = ""> {
+class BothFlags option_flags,
+list option_vis = [ClangOption],
+string help = ""> {
   list OptionFlags = option_flags;
+  list OptionVis = option_vis;
   string Help = help;
 }
 
@@ -411,23 +429,26 @@
   FlagDef Result
 = FlagDef;
 }
 
 // Definition of the command line flag with positive spelling, e.g. "-ffoo".
-class PosFlag flags = [], string help = "",
-  list implied_by_expressions = []>
-  : FlagDef {}
+class PosFlag flags = [], list vis = [],
+  string help = "", list implied_by_expressions = []>
+  : FlagDef {}
 
 // Definition of the command line flag with negative spelling, e.g. "-fno-foo".
-class NegFlag flags = [], string help = "",
-  list implied_by_expressions = []>
-  : FlagDef {}
+class NegFlag flags = [], list vis = [],
+  string help = "", list implied_by_expressions = []>
+  : FlagDef {}
 
 // Expanded FlagDef that's convenient for creation of TableGen records.
 class FlagDefExpanded
-  : FlagDef {
+  : FlagDef {
   // Name of the TableGen record.
   string RecordName = prefix # !if(flag.Polarity, "", "no_") # name;
 
@@ -445,7 +466,8 @@
 class MarshalledFlagRec
-  : Flag<["-"], flag.Spelling>, Flags, HelpText,
+  : Flag<["-"], flag.Spelling>, Flags, Vis,
+HelpText,
 MarshallingInfoBooleanFlag,
 ImpliedByAnyOf {}
@@ -459,7 +481,7 @@
 multiclass BoolOption> {
+  BothFlags suffix = BothFlags<[]>> {
   defvar flag1 = FlagDefExpanded.Result, prefix,
  NAME, spelling_base>;
 
@@ -490,7 +512,7 @@
 /// CompilerInvocation.
 multiclass BoolFOption> {
+   BothFlags both = BothFlags<[]>> {
   defm NAME : BoolOption<"f", flag_base, kpm, default, flag1, flag2, both>,
   Group;
 }
@@ -501,7 +523,7 @@
 // CompilerInvocation.
 multiclass BoolGOption> {
+   BothFlags both = BothFlags<[]>> {
   defm NAME : BoolOption<"g", flag_base, kpm, default, flag1, flag2, both>,
   Group;
 }
@@ -509,7 +531,7 @@
 // Works like BoolOption except without marshalling
 multiclass BoolOptionWithoutMarshalling> {
+BothFlags suffix = BothFlags<[]>> {
   defvar flag1 = FlagDefExpanded.Result, prefix,
  NAME, spelling_base>;
 
@@ -531,11 +553,11 @@
   defvar implied = 

[PATCH] D157149: [Option] Add "Visibility" field and clone the OptTable APIs to use it

[Justin Bogner via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGa16104e6da6f: [Option] Add Visibility field and 
clone the OptTable APIs to use it (authored by bogner).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157149/new/

https://reviews.llvm.org/D157149

Files:
  clang-tools-extra/clangd/CompileCommands.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  lld/MachO/DriverUtils.cpp
  lld/MinGW/Driver.cpp
  lld/wasm/Driver.cpp
  llvm/include/llvm/Option/OptParser.td
  llvm/include/llvm/Option/OptTable.h
  llvm/include/llvm/Option/Option.h
  llvm/lib/ExecutionEngine/JITLink/COFFDirectiveParser.cpp
  llvm/lib/Option/OptTable.cpp
  llvm/lib/ToolDrivers/llvm-dlltool/DlltoolDriver.cpp
  llvm/lib/ToolDrivers/llvm-lib/LibDriver.cpp
  llvm/tools/dsymutil/dsymutil.cpp
  llvm/tools/llvm-cvtres/llvm-cvtres.cpp
  llvm/tools/llvm-cxxfilt/llvm-cxxfilt.cpp
  llvm/tools/llvm-debuginfod/llvm-debuginfod.cpp
  llvm/tools/llvm-dwarfutil/llvm-dwarfutil.cpp
  llvm/tools/llvm-dwp/llvm-dwp.cpp
  llvm/tools/llvm-lipo/llvm-lipo.cpp
  llvm/tools/llvm-mt/llvm-mt.cpp
  llvm/tools/llvm-objcopy/ObjcopyOptions.cpp
  llvm/tools/llvm-rc/llvm-rc.cpp
  llvm/tools/llvm-strings/llvm-strings.cpp
  llvm/tools/llvm-symbolizer/llvm-symbolizer.cpp
  llvm/tools/llvm-tli-checker/llvm-tli-checker.cpp
  llvm/unittests/Option/OptionMarshallingTest.cpp
  llvm/unittests/Option/OptionParsingTest.cpp
  llvm/unittests/Option/Opts.td
  llvm/utils/TableGen/OptParserEmitter.cpp

Index: llvm/utils/TableGen/OptParserEmitter.cpp
===
--- llvm/utils/TableGen/OptParserEmitter.cpp
+++ llvm/utils/TableGen/OptParserEmitter.cpp
@@ -302,7 +302,7 @@
   OS << "INVALID";
 
 // The other option arguments (unused for groups).
-OS << ", INVALID, nullptr, 0, 0";
+OS << ", INVALID, nullptr, 0, 0, 0";
 
 // The option help text.
 if (!isa(R.getValueInit("HelpText"))) {
@@ -340,8 +340,10 @@
 // The containing option group (if any).
 OS << ", ";
 const ListInit *GroupFlags = nullptr;
+const ListInit *GroupVis = nullptr;
 if (const DefInit *DI = dyn_cast(R.getValueInit("Group"))) {
   GroupFlags = DI->getDef()->getValueAsListInit("Flags");
+  GroupVis = DI->getDef()->getValueAsListInit("Vis");
   OS << getOptionName(*DI->getDef());
 } else
   OS << "INVALID";
@@ -368,7 +370,7 @@
   OS << "\"";
 }
 
-// The option flags.
+// "Flags" for the option, such as HelpHidden and Render*
 OS << ", ";
 int NumFlags = 0;
 const ListInit *LI = R.getValueAsListInit("Flags");
@@ -382,6 +384,21 @@
 if (NumFlags == 0)
   OS << '0';
 
+// Option visibility, for sharing options between drivers.
+OS << ", ";
+int NumVisFlags = 0;
+LI = R.getValueAsListInit("Vis");
+for (Init *I : *LI)
+  OS << (NumVisFlags++ ? " | " : "")
+ << cast(I)->getDef()->getName();
+if (GroupVis) {
+  for (Init *I : *GroupVis)
+OS << (NumVisFlags++ ? " | " : "")
+   << cast(I)->getDef()->getName();
+}
+if (NumVisFlags == 0)
+  OS << '0';
+
 // The option parameter field.
 OS << ", " << R.getValueAsInt("NumArgs");
 
Index: llvm/unittests/Option/Opts.td
===
--- llvm/unittests/Option/Opts.td
+++ llvm/unittests/Option/Opts.td
@@ -4,6 +4,8 @@
 def OptFlag2 : OptionFlag;
 def OptFlag3 : OptionFlag;
 
+def SubtoolVis : OptionVisibility;
+
 def A : Flag<["-"], "A">, HelpText<"The A option">, Flags<[OptFlag1]>;
 def AB : Flag<["-"], "AB">;
 def B : Joined<["-"], "B">, HelpText<"The B option">, MetaVarName<"B">, Flags<[OptFlag2]>;
@@ -35,6 +37,8 @@
 def Cramb : Joined<["/"], "cramb:">, HelpText<"The cramb option">, MetaVarName<"CRAMB">, Flags<[OptFlag1]>;
 def Doopf1 : Flag<["-"], "doopf1">, HelpText<"The doopf1 option">, Flags<[OptFlag1]>;
 def Doopf2 : Flag<["-"], "doopf2">, HelpText<"The doopf2 option">, Flags<[OptFlag2]>;
+def Xyzzy1 : Flag<["-"], "xyzzy1">, HelpText<"The xyzzy1 option">, Vis<[SubtoolVis]>;
+def Xyzzy2 : Flag<["-"], "xyzzy2">, HelpText<"The xyzzy2 option">, Vis<[Default]>;
 def Ermgh : Joined<["--"], "ermgh">, HelpText<"The ermgh option">, MetaVarName<"ERMGH">, Flags<[OptFlag1]>;
 def Fjormp : Flag<["--"], "fjormp">, HelpText<"The fjormp option">, Flags<[OptFlag1]>;
 
@@ -43,6 +47,9 @@
 def Blurmpq : Flag<["--"], "blurmp">;
 def Blurmpq_eq : Flag<["--"], "blurmp=">;
 
+def Q : Flag<["-"], "Q">, Vis<[SubtoolVis]>;
+def R : Flag<["-"], "R">, Vis<[Default, SubtoolVis]>;
+
 class XOpts : KeyPathAndMacro<"X->", base> {}
 
 def marshalled_flag_d : Flag<["-"], "marshalled-flag-d">,
Index: llvm/unittests/Option/OptionParsingTest.cpp
===
--- llvm/unittests/Option/OptionParsingTest.cpp
+++ llvm/unittests/Option/OptionParsingTest.cpp

[PATCH] D157149: [Option] Add "Visibility" field and clone the OptTable APIs to use it

[Justin Bogner via Phabricator via cfe-commits]

bogner updated this revision to Diff 550055.
bogner edited the summary of this revision.
bogner added a comment.

Update comment wording to be less ambiguous about flags vs visibility


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157149/new/

https://reviews.llvm.org/D157149

Files:
  clang-tools-extra/clangd/CompileCommands.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  lld/MachO/DriverUtils.cpp
  lld/MinGW/Driver.cpp
  lld/wasm/Driver.cpp
  llvm/include/llvm/Option/OptParser.td
  llvm/include/llvm/Option/OptTable.h
  llvm/include/llvm/Option/Option.h
  llvm/lib/ExecutionEngine/JITLink/COFFDirectiveParser.cpp
  llvm/lib/Option/OptTable.cpp
  llvm/lib/ToolDrivers/llvm-dlltool/DlltoolDriver.cpp
  llvm/lib/ToolDrivers/llvm-lib/LibDriver.cpp
  llvm/tools/dsymutil/dsymutil.cpp
  llvm/tools/llvm-cvtres/llvm-cvtres.cpp
  llvm/tools/llvm-cxxfilt/llvm-cxxfilt.cpp
  llvm/tools/llvm-debuginfod/llvm-debuginfod.cpp
  llvm/tools/llvm-dwarfutil/llvm-dwarfutil.cpp
  llvm/tools/llvm-dwp/llvm-dwp.cpp
  llvm/tools/llvm-lipo/llvm-lipo.cpp
  llvm/tools/llvm-mt/llvm-mt.cpp
  llvm/tools/llvm-objcopy/ObjcopyOptions.cpp
  llvm/tools/llvm-rc/llvm-rc.cpp
  llvm/tools/llvm-strings/llvm-strings.cpp
  llvm/tools/llvm-symbolizer/llvm-symbolizer.cpp
  llvm/tools/llvm-tli-checker/llvm-tli-checker.cpp
  llvm/unittests/Option/OptionMarshallingTest.cpp
  llvm/unittests/Option/OptionParsingTest.cpp
  llvm/unittests/Option/Opts.td
  llvm/utils/TableGen/OptParserEmitter.cpp

Index: llvm/utils/TableGen/OptParserEmitter.cpp
===
--- llvm/utils/TableGen/OptParserEmitter.cpp
+++ llvm/utils/TableGen/OptParserEmitter.cpp
@@ -302,7 +302,7 @@
   OS << "INVALID";
 
 // The other option arguments (unused for groups).
-OS << ", INVALID, nullptr, 0, 0";
+OS << ", INVALID, nullptr, 0, 0, 0";
 
 // The option help text.
 if (!isa(R.getValueInit("HelpText"))) {
@@ -340,8 +340,10 @@
 // The containing option group (if any).
 OS << ", ";
 const ListInit *GroupFlags = nullptr;
+const ListInit *GroupVis = nullptr;
 if (const DefInit *DI = dyn_cast(R.getValueInit("Group"))) {
   GroupFlags = DI->getDef()->getValueAsListInit("Flags");
+  GroupVis = DI->getDef()->getValueAsListInit("Vis");
   OS << getOptionName(*DI->getDef());
 } else
   OS << "INVALID";
@@ -368,7 +370,7 @@
   OS << "\"";
 }
 
-// The option flags.
+// "Flags" for the option, such as HelpHidden and Render*
 OS << ", ";
 int NumFlags = 0;
 const ListInit *LI = R.getValueAsListInit("Flags");
@@ -382,6 +384,21 @@
 if (NumFlags == 0)
   OS << '0';
 
+// Option visibility, for sharing options between drivers.
+OS << ", ";
+int NumVisFlags = 0;
+LI = R.getValueAsListInit("Vis");
+for (Init *I : *LI)
+  OS << (NumVisFlags++ ? " | " : "")
+ << cast(I)->getDef()->getName();
+if (GroupVis) {
+  for (Init *I : *GroupVis)
+OS << (NumVisFlags++ ? " | " : "")
+   << cast(I)->getDef()->getName();
+}
+if (NumVisFlags == 0)
+  OS << '0';
+
 // The option parameter field.
 OS << ", " << R.getValueAsInt("NumArgs");
 
Index: llvm/unittests/Option/Opts.td
===
--- llvm/unittests/Option/Opts.td
+++ llvm/unittests/Option/Opts.td
@@ -4,6 +4,8 @@
 def OptFlag2 : OptionFlag;
 def OptFlag3 : OptionFlag;
 
+def SubtoolVis : OptionVisibility;
+
 def A : Flag<["-"], "A">, HelpText<"The A option">, Flags<[OptFlag1]>;
 def AB : Flag<["-"], "AB">;
 def B : Joined<["-"], "B">, HelpText<"The B option">, MetaVarName<"B">, Flags<[OptFlag2]>;
@@ -35,6 +37,8 @@
 def Cramb : Joined<["/"], "cramb:">, HelpText<"The cramb option">, MetaVarName<"CRAMB">, Flags<[OptFlag1]>;
 def Doopf1 : Flag<["-"], "doopf1">, HelpText<"The doopf1 option">, Flags<[OptFlag1]>;
 def Doopf2 : Flag<["-"], "doopf2">, HelpText<"The doopf2 option">, Flags<[OptFlag2]>;
+def Xyzzy1 : Flag<["-"], "xyzzy1">, HelpText<"The xyzzy1 option">, Vis<[SubtoolVis]>;
+def Xyzzy2 : Flag<["-"], "xyzzy2">, HelpText<"The xyzzy2 option">, Vis<[Default]>;
 def Ermgh : Joined<["--"], "ermgh">, HelpText<"The ermgh option">, MetaVarName<"ERMGH">, Flags<[OptFlag1]>;
 def Fjormp : Flag<["--"], "fjormp">, HelpText<"The fjormp option">, Flags<[OptFlag1]>;
 
@@ -43,6 +47,9 @@
 def Blurmpq : Flag<["--"], "blurmp">;
 def Blurmpq_eq : Flag<["--"], "blurmp=">;
 
+def Q : Flag<["-"], "Q">, Vis<[SubtoolVis]>;
+def R : Flag<["-"], "R">, Vis<[Default, SubtoolVis]>;
+
 class XOpts : KeyPathAndMacro<"X->", base> {}
 
 def marshalled_flag_d : Flag<["-"], "marshalled-flag-d">,
Index: llvm/unittests/Option/OptionParsingTest.cpp
===
--- llvm/unittests/Option/OptionParsingTest.cpp
+++ llvm/unittests/Option/OptionParsingTest.cpp
@@ -44,6 +44,10 @@
   OptFlag3 = (1 << 6)
 };
 
+enum 

[PATCH] D157794: [Driver] Make /Zi and /Z7 aliases of -g rather than handling them specially

[Justin Bogner via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGf58330cbe445: [Driver] Make /Zi and /Z7 aliases of -g rather 
than handling them specially (authored by bogner).

Changed prior to commit:
  https://reviews.llvm.org/D157794?vs=549643=550053#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157794/new/

https://reviews.llvm.org/D157794

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/Clang.h
  clang/test/Driver/cl-options.c
  clang/test/Driver/working-directory.c

Index: clang/test/Driver/working-directory.c
===
--- clang/test/Driver/working-directory.c
+++ clang/test/Driver/working-directory.c
@@ -6,6 +6,6 @@
 
 // CHECK_NO_FILE: no such file or directory: 'no_such_file.cpp'
 
+// CHECK_WORKS: "-fdebug-compilation-dir={{[^"]+}}test{{/|}}Driver{{/|}}Inputs"
 // CHECK_WORKS: "-coverage-notes-file" "{{[^"]+}}test{{/|}}Driver{{/|}}Inputs{{/|}}pchfile.gcno"
 // CHECK_WORKS: "-working-directory" "{{[^"]+}}test{{/|}}Driver{{/|}}Inputs"
-// CHECK_WORKS: "-fdebug-compilation-dir={{[^"]+}}test{{/|}}Driver{{/|}}Inputs"
Index: clang/test/Driver/cl-options.c
===
--- clang/test/Driver/cl-options.c
+++ clang/test/Driver/cl-options.c
@@ -564,16 +564,10 @@
 // RUN: %clang_cl /Brepro /Brepro- /c '-###' -- %s 2>&1 | FileCheck -check-prefix=Brepro_ %s
 // Brepro_: "-mincremental-linker-compatible"
 
-// This test was super sneaky: "/Z7" means "line-tables", but "-gdwarf" occurs
-// later on the command line, so it should win. Interestingly the cc1 arguments
-// came out right, but had wrong semantics, because an invariant assumed by
-// CompilerInvocation was violated: it expects that at most one of {gdwarfN,
-// line-tables-only} appear. If you assume that, then you can safely use
-// Args.hasArg to test whether a boolean flag is present without caring
-// where it appeared. And for this test, it appeared to the left of -gdwarf
-// which made it "win". This test could not detect that bug.
+// If We specify both /Z7 and -gdwarf we should get dwarf, not codeview.
 // RUN: %clang_cl /Z7 -gdwarf /c -### -- %s 2>&1 | FileCheck -check-prefix=Z7_gdwarf %s
-// Z7_gdwarf: "-gcodeview"
+// RUN: %clang_cl -gdwarf /Z7 /c -### -- %s 2>&1 | FileCheck -check-prefix=Z7_gdwarf %s
+// Z7_gdwarf-NOT: "-gcodeview"
 // Z7_gdwarf: "-debug-info-kind=constructor"
 // Z7_gdwarf: "-dwarf-version=
 
Index: clang/lib/Driver/ToolChains/Clang.h
===
--- clang/lib/Driver/ToolChains/Clang.h
+++ clang/lib/Driver/ToolChains/Clang.h
@@ -90,9 +90,7 @@
  RewriteKind rewrite) const;
 
   void AddClangCLArgs(const llvm::opt::ArgList , types::ID InputType,
-  llvm::opt::ArgStringList ,
-  llvm::codegenoptions::DebugInfoKind *DebugInfoKind,
-  bool *EmitCodeView) const;
+  llvm::opt::ArgStringList ) const;
 
   mutable std::unique_ptr CompilationDatabase = nullptr;
   void DumpCompilationDatabase(Compilation , StringRef Filename,
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4204,8 +4204,8 @@
 
 static void
 renderDebugOptions(const ToolChain , const Driver , const llvm::Triple ,
-   const ArgList , bool EmitCodeView, bool IRInput,
-   ArgStringList ,
+   const ArgList , bool IRInput, ArgStringList ,
+   const InputInfo ,
llvm::codegenoptions::DebugInfoKind ,
DwarfFissionKind ) {
   if (Args.hasFlag(options::OPT_fdebug_info_for_profiling,
@@ -4282,6 +4282,7 @@
   if (const Arg *A = getDwarfNArg(Args))
 EmitDwarf = checkDebugInfoOption(A, Args, D, TC);
 
+  bool EmitCodeView = false;
   if (const Arg *A = Args.getLastArg(options::OPT_gcodeview))
 EmitCodeView = checkDebugInfoOption(A, Args, D, TC);
 
@@ -4518,6 +4519,33 @@
 
   renderDwarfFormat(D, T, Args, CmdArgs, EffectiveDWARFVersion);
   RenderDebugInfoCompressionArgs(Args, CmdArgs, D, TC);
+
+  // This controls whether or not we perform JustMyCode instrumentation.
+  if (Args.hasFlag(options::OPT_fjmc, options::OPT_fno_jmc, false)) {
+if (TC.getTriple().isOSBinFormatELF() || D.IsCLMode()) {
+  if (DebugInfoKind >= llvm::codegenoptions::DebugInfoConstructor)
+CmdArgs.push_back("-fjmc");
+  else if (D.IsCLMode())
+D.Diag(clang::diag::warn_drv_jmc_requires_debuginfo) << "/JMC"
+ << "'/Zi', '/Z7'";
+  else
+

[PATCH] D157794: [Driver] Make /Zi and /Z7 aliases of -g rather than handling them specially

[Justin Bogner via Phabricator via cfe-commits]

bogner added inline comments.



Comment at: clang/test/Driver/cl-options.c:567
 
-// This test was super sneaky: "/Z7" means "line-tables", but "-gdwarf" occurs
-// later on the command line, so it should win. Interestingly the cc1 arguments
-// came out right, but had wrong semantics, because an invariant assumed by
-// CompilerInvocation was violated: it expects that at most one of {gdwarfN,
-// line-tables-only} appear. If you assume that, then you can safely use
-// Args.hasArg to test whether a boolean flag is present without caring
-// where it appeared. And for this test, it appeared to the left of -gdwarf
-// which made it "win". This test could not detect that bug.
+// If We specify both /Z7 and -gdwarf we should get dwarf, not codeview.
 // RUN: %clang_cl /Z7 -gdwarf /c -### -- %s 2>&1 | FileCheck 
-check-prefix=Z7_gdwarf %s

smeenai wrote:
> bogner wrote:
> > rnk wrote:
> > > I think Meta folks depend on a mode that emits both codeview and DWARF. 
> > > +@smeenai
> > Hopefully if that's the case there's a better test somewhere than this one 
> > that discusses in detail how `/Z7` is an alias for `-gline-tables-only`, 
> > which hasn't been true since 2017.
> Thanks for the headers up :) We don't depend on that mode any more though, so 
> no problems on our end.
Also note that this change does not affect what happens if we specify both 
`-gcodeview` and `-gdwarf` together. That continues to pass both `-gcodeview` 
and `-dwarf-version=...` to `clang -cc1`. However, I don't see any tests that 
actually check that behaviour, so if that is a mode that's useful to keep 
working we definitely have some gaps there.

This change means that if you want to ask `-cc1` for dwarf *and* codeview with 
`/Z7` or `/Zi`, you'll need to specify `-gdwarf` and `-gcodeview` now. This 
matches how you would get both sets of options to render with `-g`, which I 
think makes sense.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157794/new/

https://reviews.llvm.org/D157794

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D157837: [flang][driver] Update the visibility of Clang options in Flang

[Justin Bogner via Phabricator via cfe-commits]

bogner added a comment.

I can't speak to which flags should be present in flang-new or not, but in 
principle this looks great. You'll need to update the patch to use the 
"ClangOption" spelling rather than "Default" of course, as discussed in 
https://reviews.llvm.org/D157151




Comment at: clang/lib/Driver/Driver.cpp:6473
 // TODO: Does flang really want *all* of the clang driver options?
 // We probably need to annotate more specifically.
+return llvm::opt::Visibility(options::FlangOption);

We can remove the TODO comment with this change :)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157837/new/

https://reviews.llvm.org/D157837

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D157151: [Driver] Refactor to use llvm Option's new Visibility flags

[Justin Bogner via Phabricator via cfe-commits]

bogner added a comment.

In D157151#4583904 , @awarzynski 
wrote:

> This can be tweaked in `getOptionVisibilityMask` (extracted from this patch):
>
>   llvm::opt::Visibility
>   Driver::getOptionVisibilityMask(bool UseDriverMode) const {
> if (!UseDriverMode)
>   return llvm::opt::Visibility(llvm::opt::Default);
> if (IsCLMode())
>   return llvm::opt::Visibility(options::CLOption);
> if (IsDXCMode())
>   return llvm::opt::Visibility(options::DXCOption);
> if (IsFlangMode()) {
>   // vvv HERE vvv
>   // TODO: Does flang really want *all* of the clang driver options?
>   // We probably need to annotate more specifically.
>   return llvm::opt::Visibility(llvm::opt::Default | options::FlangOption);
> }
> return llvm::opt::Visibility(llvm::opt::Default);
>   }
>
> Now, prior to this change I couldn't find a way to disable unsupported Clang 
> options in Flang. With this patch,
>
> - all Clang options gain a visibility flag (atm that's `Default`),
> - that flag can be used to disable options unsupported in Flang.
>
> For this to work, all options supported by Flang need to have their 
> visibility flags set accordingly. That's the goal, but I can see that we have 
> missed quite a few options over the time (*). Updated here: 
> https://reviews.llvm.org/D157837.

Ah right. Yes, this is exactly the problem that this patch is trying to fix. 
Glad to see that it seems to be working as intended for flang-new!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157151/new/

https://reviews.llvm.org/D157151

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D157150: [Driver] Update BoolOption to handle Visibility. NFC

[Justin Bogner via Phabricator via cfe-commits]

bogner updated this revision to Diff 549853.
bogner added a comment.

Use "ClangOption" rather than "Default" in Options.td, as per the conversation 
in https://reviews.llvm.org/D157151


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157150/new/

https://reviews.llvm.org/D157150

Files:
  clang/include/clang/Driver/Options.h
  clang/include/clang/Driver/Options.td

Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -79,6 +79,23 @@
 // target will lead to an err_drv_unsupported_opt_for_target error.
 def TargetSpecific : OptionFlag;
 
+// Indicates that this warning is ignored, but accepted with a warning for
+// GCC compatibility.
+class IgnoredGCCCompat : Flags<[HelpHidden]> {}
+
+class TargetSpecific : Flags<[TargetSpecific]> {}
+
+/
+// Visibility
+
+// We prefer the name "ClangOption" here rather than "Default" to make
+// it clear that these options will be visible in the clang driver (as
+// opposed to clang -cc1, the CL driver, or the flang driver).
+defvar ClangOption = Default;
+
+/
+// Docs
+
 // A short name to show in documentation. The name will be interpreted as rST.
 class DocName { string DocName = name; }
 
@@ -89,12 +106,6 @@
 // documentation.
 class DocFlatten { bit DocFlatten = 1; }
 
-// Indicates that this warning is ignored, but accepted with a warning for
-// GCC compatibility.
-class IgnoredGCCCompat : Flags<[HelpHidden]> {}
-
-class TargetSpecific : Flags<[TargetSpecific]> {}
-
 /
 // Groups
 
@@ -381,7 +392,8 @@
 
 // Definition of single command line flag. This is an implementation detail, use
 // SetTrueBy or SetFalseBy instead.
-class FlagDef option_flags,
+class FlagDef option_flags, list option_vis,
   string help, list implied_by_expressions = []> {
   // The polarity. Besides spelling, this also decides whether the TableGen
   // record will be prefixed with "no_".
@@ -390,9 +402,12 @@
   // The value assigned to key path when the flag is present on command line.
   bit Value = value;
 
-  // OptionFlags that control visibility of the flag in different tools.
+  // OptionFlags in different tools.
   list OptionFlags = option_flags;
 
+  // OptionVisibility flags for different tools.
+  list OptionVis = option_vis;
+
   // The help text associated with the flag.
   string Help = help;
 
@@ -401,8 +416,11 @@
 }
 
 // Additional information to be appended to both positive and negative flag.
-class BothFlags option_flags, string help = ""> {
+class BothFlags option_flags,
+list option_vis = [ClangOption],
+string help = ""> {
   list OptionFlags = option_flags;
+  list OptionVis = option_vis;
   string Help = help;
 }
 
@@ -411,23 +429,26 @@
   FlagDef Result
 = FlagDef;
 }
 
 // Definition of the command line flag with positive spelling, e.g. "-ffoo".
-class PosFlag flags = [], string help = "",
-  list implied_by_expressions = []>
-  : FlagDef {}
+class PosFlag flags = [], list vis = [],
+  string help = "", list implied_by_expressions = []>
+  : FlagDef {}
 
 // Definition of the command line flag with negative spelling, e.g. "-fno-foo".
-class NegFlag flags = [], string help = "",
-  list implied_by_expressions = []>
-  : FlagDef {}
+class NegFlag flags = [], list vis = [],
+  string help = "", list implied_by_expressions = []>
+  : FlagDef {}
 
 // Expanded FlagDef that's convenient for creation of TableGen records.
 class FlagDefExpanded
-  : FlagDef {
+  : FlagDef {
   // Name of the TableGen record.
   string RecordName = prefix # !if(flag.Polarity, "", "no_") # name;
 
@@ -445,7 +466,8 @@
 class MarshalledFlagRec
-  : Flag<["-"], flag.Spelling>, Flags, HelpText,
+  : Flag<["-"], flag.Spelling>, Flags, Vis,
+HelpText,
 MarshallingInfoBooleanFlag,
 ImpliedByAnyOf {}
@@ -459,7 +481,7 @@
 multiclass BoolOption> {
+  BothFlags suffix = BothFlags<[]>> {
   defvar flag1 = FlagDefExpanded.Result, prefix,
  NAME, spelling_base>;
 
@@ -490,7 +512,7 @@
 /// CompilerInvocation.
 multiclass BoolFOption> {
+   BothFlags both = BothFlags<[]>> {
   defm NAME : BoolOption<"f", flag_base, kpm, default, flag1, flag2, both>,
   Group;
 }
@@ -501,7 +523,7 @@
 // CompilerInvocation.
 multiclass BoolGOption> {
+   BothFlags both = BothFlags<[]>> {
   defm NAME : BoolOption<"g", flag_base, kpm, default, flag1, flag2, both>,
   Group;
 }
@@ -509,7 +531,7 @@
 // Works like BoolOption except without marshalling
 multiclass BoolOptionWithoutMarshalling> {
+BothFlags suffix = BothFlags<[]>> {
   defvar flag1 = FlagDefExpanded.Result, prefix,
  NAME, spelling_base>;
 
@@ -531,11 +553,11 @@

[PATCH] D157146: [Clang][Docs] Consolidate option hiding in ClangOptionDocEmitter

[Justin Bogner via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGce16c3cf30f5: [Clang][Docs] Consolidate option hiding in 
ClangOptionDocEmitter (authored by bogner).

Changed prior to commit:
  https://reviews.llvm.org/D157146?vs=547371=549805#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157146/new/

https://reviews.llvm.org/D157146

Files:
  clang/utils/TableGen/ClangOptionDocEmitter.cpp

Index: clang/utils/TableGen/ClangOptionDocEmitter.cpp
===
--- clang/utils/TableGen/ClangOptionDocEmitter.cpp
+++ clang/utils/TableGen/ClangOptionDocEmitter.cpp
@@ -31,13 +31,41 @@
 struct Documentation {
   std::vector Groups;
   std::vector Options;
+
+  bool empty() {
+return Groups.empty() && Options.empty();
+  }
 };
 struct DocumentedGroup : Documentation {
   Record *Group;
 };
 
+static bool hasFlag(const Record *Option, StringRef OptionFlag) {
+  for (const Record *Flag : Option->getValueAsListOfDefs("Flags"))
+if (Flag->getName() == OptionFlag)
+  return true;
+  if (const DefInit *DI = dyn_cast(Option->getValueInit("Group")))
+for (const Record *Flag : DI->getDef()->getValueAsListOfDefs("Flags"))
+  if (Flag->getName() == OptionFlag)
+return true;
+  return false;
+}
+
+static bool isOptionVisible(const Record *Option, const Record *DocInfo) {
+  for (StringRef Exclusion : DocInfo->getValueAsListOfStrings("ExcludedFlags"))
+if (hasFlag(Option, Exclusion))
+  return false;
+  if (!DocInfo->getValue("IncludedFlags"))
+return true;
+  for (StringRef Inclusion : DocInfo->getValueAsListOfStrings("IncludedFlags"))
+if (hasFlag(Option, Inclusion))
+  return true;
+  return false;
+}
+
 // Reorganize the records into a suitable form for emitting documentation.
-Documentation extractDocumentation(RecordKeeper ) {
+Documentation extractDocumentation(RecordKeeper ,
+   const Record *DocInfo) {
   Documentation Result;
 
   // Build the tree of groups. The root in the tree is the fake option group
@@ -124,12 +152,15 @@
   D.Groups.back().Group = G;
   Documentation  = D.Groups.back();
   Base = DocumentationForGroup(G);
+  if (Base.empty())
+D.Groups.pop_back();
 }
 
 auto  = OptionsInGroup[R];
 llvm::sort(Options, CompareByName);
 for (Record *O : Options)
-  D.Options.push_back(DocumentationForOption(O));
+  if (isOptionVisible(O, DocInfo))
+D.Options.push_back(DocumentationForOption(O));
 
 return D;
   };
@@ -161,44 +192,6 @@
 .Default(0);
 }
 
-bool hasFlag(const Record *OptionOrGroup, StringRef OptionFlag) {
-  for (const Record *Flag : OptionOrGroup->getValueAsListOfDefs("Flags"))
-if (Flag->getName() == OptionFlag)
-  return true;
-  return false;
-}
-
-bool isIncluded(const Record *OptionOrGroup, const Record *DocInfo) {
-  assert(DocInfo->getValue("IncludedFlags") && "Missing includeFlags");
-  for (StringRef Inclusion : DocInfo->getValueAsListOfStrings("IncludedFlags"))
-if (hasFlag(OptionOrGroup, Inclusion))
-  return true;
-  return false;
-}
-
-bool isGroupIncluded(const DocumentedGroup , const Record *DocInfo) {
-  if (isIncluded(Group.Group, DocInfo))
-return true;
-  for (auto  : Group.Options)
-if (isIncluded(O.Option, DocInfo))
-  return true;
-  for (auto  : Group.Groups) {
-if (isIncluded(G.Group, DocInfo))
-  return true;
-if (isGroupIncluded(G, DocInfo))
-  return true;
-  }
-  return false;
-}
-
-bool isExcluded(const Record *OptionOrGroup, const Record *DocInfo) {
-  // FIXME: Provide a flag to specify the set of exclusions.
-  for (StringRef Exclusion : DocInfo->getValueAsListOfStrings("ExcludedFlags"))
-if (hasFlag(OptionOrGroup, Exclusion))
-  return true;
-  return false;
-}
-
 std::string escapeRST(StringRef Str) {
   std::string Out;
   for (auto K : Str) {
@@ -319,16 +312,13 @@
   F(Option.Option);
 
   for (auto *Alias : Option.Aliases)
-if (!isExcluded(Alias, DocInfo) && canSphinxCopeWithOption(Option.Option))
+if (isOptionVisible(Alias, DocInfo) &&
+canSphinxCopeWithOption(Option.Option))
   F(Alias);
 }
 
 void emitOption(const DocumentedOption , const Record *DocInfo,
 raw_ostream ) {
-  if (isExcluded(Option.Option, DocInfo))
-return;
-  if (DocInfo->getValue("IncludedFlags") && !isIncluded(Option.Option, DocInfo))
-return;
   if (Option.Option->getValueAsDef("Kind")->getName() == "KIND_UNKNOWN" ||
   Option.Option->getValueAsDef("Kind")->getName() == "KIND_INPUT")
 return;
@@ -401,12 +391,6 @@
 
 void emitGroup(int Depth, const DocumentedGroup , const Record *DocInfo,
raw_ostream ) {
-  if (isExcluded(Group.Group, DocInfo))
-return;
-
-  if (DocInfo->getValue("IncludedFlags") && !isGroupIncluded(Group, DocInfo))
-

[PATCH] D157794: [Driver] Make /Zi and /Z7 aliases of -g rather than handling them specially

[Justin Bogner via Phabricator via cfe-commits]

bogner added inline comments.



Comment at: clang/test/Driver/cl-options.c:567
 
-// This test was super sneaky: "/Z7" means "line-tables", but "-gdwarf" occurs
-// later on the command line, so it should win. Interestingly the cc1 arguments
-// came out right, but had wrong semantics, because an invariant assumed by
-// CompilerInvocation was violated: it expects that at most one of {gdwarfN,
-// line-tables-only} appear. If you assume that, then you can safely use
-// Args.hasArg to test whether a boolean flag is present without caring
-// where it appeared. And for this test, it appeared to the left of -gdwarf
-// which made it "win". This test could not detect that bug.
+// If We specify both /Z7 and -gdwarf we should get dwarf, not codeview.
 // RUN: %clang_cl /Z7 -gdwarf /c -### -- %s 2>&1 | FileCheck 
-check-prefix=Z7_gdwarf %s

rnk wrote:
> I think Meta folks depend on a mode that emits both codeview and DWARF. 
> +@smeenai
Hopefully if that's the case there's a better test somewhere than this one that 
discusses in detail how `/Z7` is an alias for `-gline-tables-only`, which 
hasn't been true since 2017.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157794/new/

https://reviews.llvm.org/D157794

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D157794: [Driver] Make /Zi and /Z7 aliases of -g rather than handling them specially

[Justin Bogner via Phabricator via cfe-commits]

bogner created this revision.
bogner added reviewers: rnk, jansvoboda11, MaskRay.
Herald added subscribers: jeroen.dobbelaere, mcrosier.
Herald added a project: All.
bogner requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

The -g flag has been selecting whether to emit dwarf or codeview based
on the target ABI since 2018, so simply aliasing these flags does the
right thing for clang-cl.

This moves some code from Clang::ConstructJob to renderDebugOptions to
make things a little clearer now that we don't need to keep track of
whether we're doing codeview or not in multiple places, and also
combines the duplicate handling of the cl vs clang handling of jmc
flags as a result.

This is mostly NFC, but some -cc1 flags may be rendered in a slightly
different order because of the code that was moved around.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D157794

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/Clang.h
  clang/test/Driver/cl-options.c
  clang/test/Driver/working-directory.c

Index: clang/test/Driver/working-directory.c
===
--- clang/test/Driver/working-directory.c
+++ clang/test/Driver/working-directory.c
@@ -6,6 +6,6 @@
 
 // CHECK_NO_FILE: no such file or directory: 'no_such_file.cpp'
 
+// CHECK_WORKS: "-fdebug-compilation-dir={{[^"]+}}test{{/|}}Driver{{/|}}Inputs"
 // CHECK_WORKS: "-coverage-notes-file" "{{[^"]+}}test{{/|}}Driver{{/|}}Inputs{{/|}}pchfile.gcno"
 // CHECK_WORKS: "-working-directory" "{{[^"]+}}test{{/|}}Driver{{/|}}Inputs"
-// CHECK_WORKS: "-fdebug-compilation-dir={{[^"]+}}test{{/|}}Driver{{/|}}Inputs"
Index: clang/test/Driver/cl-options.c
===
--- clang/test/Driver/cl-options.c
+++ clang/test/Driver/cl-options.c
@@ -564,16 +564,10 @@
 // RUN: %clang_cl /Brepro /Brepro- /c '-###' -- %s 2>&1 | FileCheck -check-prefix=Brepro_ %s
 // Brepro_: "-mincremental-linker-compatible"
 
-// This test was super sneaky: "/Z7" means "line-tables", but "-gdwarf" occurs
-// later on the command line, so it should win. Interestingly the cc1 arguments
-// came out right, but had wrong semantics, because an invariant assumed by
-// CompilerInvocation was violated: it expects that at most one of {gdwarfN,
-// line-tables-only} appear. If you assume that, then you can safely use
-// Args.hasArg to test whether a boolean flag is present without caring
-// where it appeared. And for this test, it appeared to the left of -gdwarf
-// which made it "win". This test could not detect that bug.
+// If We specify both /Z7 and -gdwarf we should get dwarf, not codeview.
 // RUN: %clang_cl /Z7 -gdwarf /c -### -- %s 2>&1 | FileCheck -check-prefix=Z7_gdwarf %s
-// Z7_gdwarf: "-gcodeview"
+// RUN: %clang_cl -gdwarf /Z7 /c -### -- %s 2>&1 | FileCheck -check-prefix=Z7_gdwarf %s
+// Z7_gdwarf-NOT: "-gcodeview"
 // Z7_gdwarf: "-debug-info-kind=constructor"
 // Z7_gdwarf: "-dwarf-version=
 
Index: clang/lib/Driver/ToolChains/Clang.h
===
--- clang/lib/Driver/ToolChains/Clang.h
+++ clang/lib/Driver/ToolChains/Clang.h
@@ -90,9 +90,7 @@
  RewriteKind rewrite) const;
 
   void AddClangCLArgs(const llvm::opt::ArgList , types::ID InputType,
-  llvm::opt::ArgStringList ,
-  llvm::codegenoptions::DebugInfoKind *DebugInfoKind,
-  bool *EmitCodeView) const;
+  llvm::opt::ArgStringList ) const;
 
   mutable std::unique_ptr CompilationDatabase = nullptr;
   void DumpCompilationDatabase(Compilation , StringRef Filename,
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4204,8 +4204,8 @@
 
 static void
 renderDebugOptions(const ToolChain , const Driver , const llvm::Triple ,
-   const ArgList , bool EmitCodeView, bool IRInput,
-   ArgStringList ,
+   const ArgList , bool IRInput, ArgStringList ,
+   const InputInfo ,
llvm::codegenoptions::DebugInfoKind ,
DwarfFissionKind ) {
   if (Args.hasFlag(options::OPT_fdebug_info_for_profiling,
@@ -4282,6 +4282,7 @@
   if (const Arg *A = getDwarfNArg(Args))
 EmitDwarf = checkDebugInfoOption(A, Args, D, TC);
 
+  bool EmitCodeView = false;
   if (const Arg *A = Args.getLastArg(options::OPT_gcodeview))
 EmitCodeView = checkDebugInfoOption(A, Args, D, TC);
 
@@ -4518,6 +4519,34 @@
 
   renderDwarfFormat(D, T, Args, CmdArgs, EffectiveDWARFVersion);
   RenderDebugInfoCompressionArgs(Args, CmdArgs, D, TC);
+
+
+  // This controls whether or not we perform JustMyCode 

[PATCH] D157151: [Driver] Refactor to use llvm Option's new Visibility flags

[Justin Bogner via Phabricator via cfe-commits]

bogner added a comment.

In D157151#4582109 , @awarzynski 
wrote:

> I think that it would be good to replace `Default` with e.g.
>
> - `Clang`, or
> - `ClangDriver`, or
> - `ClangCompilerDriver`.
>
> Or, at least, to make the meaning of `Default` much clearer (through e.g. 
> comments). In general, I feel that `Default` is skewed towards this notion 
> that `clang` (the compiler driver tool) is the main client of `clangDriver`. 
> That used to be the case, but with Flang's driver also implemented in terms 
> of `clangDriver` , that's no longer the case. Also, the long term goal is to 
> extract the driver library out of Clang. One day :)

This is a little bit complicated by the fact that the Option library is already 
partially extracted out of clang (and used for other drivers, like lld and 
lldb). The "Default" visibility is defined in llvm's Option library, so calling 
it something like "Clang" would be pretty confusing for users that aren't the 
clang Driver library. I suppose one option would be to throw something like 
`using ClangDriver = llvm::Driver::Default;` in Options.h inside the 
`clang::driver::options` namespace, and then using the alias in Options.td. 
Would that help?

> Also, note that `Default` options will be available in both `clang` and 
> `flang-new`. That's the case today (so not something affected by your 
> changes). Ideally, Flang should be able to disable those _Clang options_. 
> That's not possible ATM. Contrary to Flang, Clang can disable _Flang options_ 
> with `FlangOnlyOption`. There is no such flag for Flang ATM, but I think that 
> we could re-use `Default` for that. WDYT?

`Default` options are either options that don't specify `Vis` at all or 
explicitly mention `Default`. They are not visible in `flang-new` after this 
change unless they also specify `FlangOption`. Specifically, instead of having 
to deal with `FlangOnlyOption` we explicitly opt in to default. Some examples:

  def default : Flag<["-"], "a">; //visible only in invocations of `clang`
  def default_explicit : Flag<["-"], "b">, Vis<[Default]>; //visible only in 
invocations of `clang`
  def flang_only : Flag<["-"], "c">, Vis<[FlangOption]>; // only `flang-new`
  def flang_and_clang : Flag<["-"], "d">, Vis<[Default, FlangOption]>; // Both 
`clang` and `flang-new`
  def flang_fc1 : Flag<["-"], "e">, Vis<[FC1Option]>; // `flang-new -fc1`, but 
not the `flang-new` driver or `clang`


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157151/new/

https://reviews.llvm.org/D157151

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D157151: [Driver] Refactor to use llvm Option's new Visibility flags

[Justin Bogner via Phabricator via cfe-commits]

bogner added a comment.

In D157151#4577790 , @awarzynski 
wrote:

> Hey @bogner , I've only skimmed through so far and it's looking great! That 
> Include/Exclude API was not fun to use. What you are proposing here takes 
> Options.td to a much a better place - this is a much needed and long overdue 
> refactor.
>
> There's quite a bit of churn here, so I will need a few days to scan. In the 
> meantime, could you update flang/docs/FlangDriver.md? And in general, please 
> note that this updates (primarily) `clangDriver` logic, which is used by both 
> by Clang and Flang. In particular, Options.td is shared. I think that it's 
> worth highlighting that this change benefits both sub-projects.

Yep, sorry about that. I tried to break this up as much as I could but given 
the number of places that depend on Options.td it was tricky. I do think this 
should makes things much easier for flang's driver in particular, as well as 
making clang-cl and clang-dxc easier to deal with.

Sorry I missed updating FlangDriver.md - FWIW I did build flang and run those 
tests with this change. I'll update the patch with doc updates momentarily.

>> introduced in llvm Option
>
> Could you add a link?

I've updated the description to mention https://reviews.llvm.org/D157149.




Comment at: clang/include/clang/Driver/Options.h:25-37
-  CoreOption = (1 << 8),
-  CLOption = (1 << 9),
-  CC1Option = (1 << 10),
-  CC1AsOption = (1 << 11),
-  NoDriverOption = (1 << 12),
-  LinkOption = (1 << 13),
-  FlangOption = (1 << 14),

awarzynski wrote:
> What happens to `CoreOption`s? Same for `NoDriverOption`s?
- `CoreOption` meant "available in clang and clang-cl (and maybe dxc...)", so 
it's now `[Default, CLOption]`.
- `NoDriverOption` is `!Default` - negative flags were a big part of why the 
include/exclude thing was awkward, so the change in 
https://reviews.llvm.org/D157149 introduced a bit for options where you don't 
say anything specific, which meant the clang driver in practice.

For a complete map, see `flag_to_vis` in the `update_options_td_flags.py` 
helper script I provided for downstreams: 
https://reviews.llvm.org/D157151#change-FMUYy4yRDQLQ



Comment at: clang/include/clang/Driver/Options.td:193
 def m_x86_Features_Group : OptionGroup<"">,
-   Group, Flags<[CoreOption]>, DocName<"X86">;
+   Group, Vis<[Default, CLOption, DXCOption]>,
+   DocName<"X86">;

awarzynski wrote:
> What's `Default` in `Vis<[Default, CLOption, DXCOption]>,`?
If you don't specify any visibility, the visibility is "Default" (See 
https://reviews.llvm.org/D157149). For all intents and purposes this means 
"Clang Driver" in this file.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157151/new/

https://reviews.llvm.org/D157151

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D157149: [Option] Add "Visibility" field and clone the OptTable APIs to use it

[Justin Bogner via Phabricator via cfe-commits]

bogner updated this revision to Diff 549111.
bogner added a comment.

Resolve conflicts


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157149/new/

https://reviews.llvm.org/D157149

Files:
  clang-tools-extra/clangd/CompileCommands.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  lld/MachO/DriverUtils.cpp
  lld/MinGW/Driver.cpp
  lld/wasm/Driver.cpp
  llvm/include/llvm/Option/OptParser.td
  llvm/include/llvm/Option/OptTable.h
  llvm/include/llvm/Option/Option.h
  llvm/lib/ExecutionEngine/JITLink/COFFDirectiveParser.cpp
  llvm/lib/Option/OptTable.cpp
  llvm/lib/ToolDrivers/llvm-dlltool/DlltoolDriver.cpp
  llvm/lib/ToolDrivers/llvm-lib/LibDriver.cpp
  llvm/tools/dsymutil/dsymutil.cpp
  llvm/tools/llvm-cvtres/llvm-cvtres.cpp
  llvm/tools/llvm-cxxfilt/llvm-cxxfilt.cpp
  llvm/tools/llvm-debuginfod/llvm-debuginfod.cpp
  llvm/tools/llvm-dwarfutil/llvm-dwarfutil.cpp
  llvm/tools/llvm-dwp/llvm-dwp.cpp
  llvm/tools/llvm-lipo/llvm-lipo.cpp
  llvm/tools/llvm-mt/llvm-mt.cpp
  llvm/tools/llvm-objcopy/ObjcopyOptions.cpp
  llvm/tools/llvm-rc/llvm-rc.cpp
  llvm/tools/llvm-strings/llvm-strings.cpp
  llvm/tools/llvm-symbolizer/llvm-symbolizer.cpp
  llvm/tools/llvm-tli-checker/llvm-tli-checker.cpp
  llvm/unittests/Option/OptionMarshallingTest.cpp
  llvm/unittests/Option/OptionParsingTest.cpp
  llvm/unittests/Option/Opts.td
  llvm/utils/TableGen/OptParserEmitter.cpp

Index: llvm/utils/TableGen/OptParserEmitter.cpp
===
--- llvm/utils/TableGen/OptParserEmitter.cpp
+++ llvm/utils/TableGen/OptParserEmitter.cpp
@@ -302,7 +302,7 @@
   OS << "INVALID";
 
 // The other option arguments (unused for groups).
-OS << ", INVALID, nullptr, 0, 0";
+OS << ", INVALID, nullptr, 0, 0, 0";
 
 // The option help text.
 if (!isa(R.getValueInit("HelpText"))) {
@@ -340,8 +340,10 @@
 // The containing option group (if any).
 OS << ", ";
 const ListInit *GroupFlags = nullptr;
+const ListInit *GroupVis = nullptr;
 if (const DefInit *DI = dyn_cast(R.getValueInit("Group"))) {
   GroupFlags = DI->getDef()->getValueAsListInit("Flags");
+  GroupVis = DI->getDef()->getValueAsListInit("Vis");
   OS << getOptionName(*DI->getDef());
 } else
   OS << "INVALID";
@@ -382,6 +384,21 @@
 if (NumFlags == 0)
   OS << '0';
 
+// The option visibility flags.
+OS << ", ";
+int NumVisFlags = 0;
+LI = R.getValueAsListInit("Vis");
+for (Init *I : *LI)
+  OS << (NumVisFlags++ ? " | " : "")
+ << cast(I)->getDef()->getName();
+if (GroupVis) {
+  for (Init *I : *GroupVis)
+OS << (NumVisFlags++ ? " | " : "")
+   << cast(I)->getDef()->getName();
+}
+if (NumVisFlags == 0)
+  OS << '0';
+
 // The option parameter field.
 OS << ", " << R.getValueAsInt("NumArgs");
 
Index: llvm/unittests/Option/Opts.td
===
--- llvm/unittests/Option/Opts.td
+++ llvm/unittests/Option/Opts.td
@@ -4,6 +4,8 @@
 def OptFlag2 : OptionFlag;
 def OptFlag3 : OptionFlag;
 
+def SubtoolVis : OptionVisibility;
+
 def A : Flag<["-"], "A">, HelpText<"The A option">, Flags<[OptFlag1]>;
 def AB : Flag<["-"], "AB">;
 def B : Joined<["-"], "B">, HelpText<"The B option">, MetaVarName<"B">, Flags<[OptFlag2]>;
@@ -35,6 +37,8 @@
 def Cramb : Joined<["/"], "cramb:">, HelpText<"The cramb option">, MetaVarName<"CRAMB">, Flags<[OptFlag1]>;
 def Doopf1 : Flag<["-"], "doopf1">, HelpText<"The doopf1 option">, Flags<[OptFlag1]>;
 def Doopf2 : Flag<["-"], "doopf2">, HelpText<"The doopf2 option">, Flags<[OptFlag2]>;
+def Xyzzy1 : Flag<["-"], "xyzzy1">, HelpText<"The xyzzy1 option">, Vis<[SubtoolVis]>;
+def Xyzzy2 : Flag<["-"], "xyzzy2">, HelpText<"The xyzzy2 option">, Vis<[Default]>;
 def Ermgh : Joined<["--"], "ermgh">, HelpText<"The ermgh option">, MetaVarName<"ERMGH">, Flags<[OptFlag1]>;
 def Fjormp : Flag<["--"], "fjormp">, HelpText<"The fjormp option">, Flags<[OptFlag1]>;
 
@@ -43,6 +47,9 @@
 def Blurmpq : Flag<["--"], "blurmp">;
 def Blurmpq_eq : Flag<["--"], "blurmp=">;
 
+def Q : Flag<["-"], "Q">, Vis<[SubtoolVis]>;
+def R : Flag<["-"], "R">, Vis<[Default, SubtoolVis]>;
+
 class XOpts : KeyPathAndMacro<"X->", base> {}
 
 def marshalled_flag_d : Flag<["-"], "marshalled-flag-d">,
Index: llvm/unittests/Option/OptionParsingTest.cpp
===
--- llvm/unittests/Option/OptionParsingTest.cpp
+++ llvm/unittests/Option/OptionParsingTest.cpp
@@ -44,6 +44,10 @@
   OptFlag3 = (1 << 6)
 };
 
+enum OptionVisibility {
+  SubtoolVis = (1 << 2),
+};
+
 static constexpr OptTable::Info InfoTable[] = {
 #define OPTION(...) LLVM_CONSTRUCT_OPT_INFO(__VA_ARGS__),
 #include "Opts.inc"
@@ -163,6 +167,43 @@
   EXPECT_EQ("bar", AL.getLastArgValue(OPT_C));
 }
 
+TYPED_TEST(OptTableTest, ParseWithVisibility) {
+  TypeParam T;
+  unsigned MAI, MAC;
+
+  const char *STArgs[] = 

[PATCH] D157562: [Driver][DXC] Treat stdin as HLSL

[Justin Bogner via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG0d459b71dc28: [Driver][DXC] Treat stdin as HLSL (authored by 
bogner).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157562/new/

https://reviews.llvm.org/D157562

Files:
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/dxc_stdin.hlsl


Index: clang/test/Driver/dxc_stdin.hlsl
===
--- /dev/null
+++ clang/test/Driver/dxc_stdin.hlsl
@@ -0,0 +1,3 @@
+// RUN: %clang_dxc -Tlib_6_7 - -### 2>&1 | FileCheck %s
+// CHECK: "-cc1"
+// CHECK-SAME: "-x" "hlsl" "-"
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -2648,6 +2648,8 @@
 if (memcmp(Value, "-", 2) == 0) {
   if (IsFlangMode()) {
 Ty = types::TY_Fortran;
+  } else if (IsDXCMode()) {
+Ty = types::TY_HLSL;
   } else {
 // If running with -E, treat as a C input (this changes the
 // builtin macros, for example). This may be overridden by -ObjC


Index: clang/test/Driver/dxc_stdin.hlsl
===
--- /dev/null
+++ clang/test/Driver/dxc_stdin.hlsl
@@ -0,0 +1,3 @@
+// RUN: %clang_dxc -Tlib_6_7 - -### 2>&1 | FileCheck %s
+// CHECK: "-cc1"
+// CHECK-SAME: "-x" "hlsl" "-"
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -2648,6 +2648,8 @@
 if (memcmp(Value, "-", 2) == 0) {
   if (IsFlangMode()) {
 Ty = types::TY_Fortran;
+  } else if (IsDXCMode()) {
+Ty = types::TY_HLSL;
   } else {
 // If running with -E, treat as a C input (this changes the
 // builtin macros, for example). This may be overridden by -ObjC
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D157582: [Driver][DXC] Handle -Fo and -Fc flags

[Justin Bogner via Phabricator via cfe-commits]

bogner created this revision.
bogner added reviewers: beanz, python3kgae.
Herald added subscribers: Anastasia, hiraditya, mcrosier.
Herald added a project: All.
bogner requested review of this revision.
Herald added subscribers: llvm-commits, cfe-commits, MaskRay.
Herald added projects: clang, LLVM.

This splits the backend and assemble actions for HLSL inputs and
handles the options in GetNamedOutputPath instead of aliasing `-o`.
This also moves how we default to emitting asm to stdout, since doing
this in the HLSL toolchain rather than the driver pollutes how the
clang driver works as well.

When both options are specified we disable collapsing the assemble
action and attempt to generate both outputs. Note that while this
handles the driver aspects, we can't actually run in that mode for now
since -cc1as doesn't understand DXIL as an input yet.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D157582

Files:
  clang/include/clang/Driver/Options.td
  clang/include/clang/Driver/Types.def
  clang/include/clang/Driver/Types.h
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/HLSL.cpp
  clang/lib/Driver/Types.cpp
  clang/test/Driver/dxc_dxv_path.hlsl
  clang/test/Driver/dxc_output.hlsl
  llvm/lib/MC/MCParser/AsmParser.cpp

Index: llvm/lib/MC/MCParser/AsmParser.cpp
===
--- llvm/lib/MC/MCParser/AsmParser.cpp
+++ llvm/lib/MC/MCParser/AsmParser.cpp
@@ -807,7 +807,7 @@
 PlatformParser.reset(createXCOFFAsmParser());
 break;
   case MCContext::IsDXContainer:
-llvm_unreachable("DXContainer is not supported yet");
+report_fatal_error("DXContainer is not supported yet");
 break;
   }
 
Index: clang/test/Driver/dxc_output.hlsl
===
--- /dev/null
+++ clang/test/Driver/dxc_output.hlsl
@@ -0,0 +1,42 @@
+// With no output args, we emit assembly to stdout.
+// RUN: %clang_dxc -T lib_6_7 -Vd %s -### 2>&1 | FileCheck %s --check-prefixes=CHECK-CC1,CHECK-CC1-STDOUT
+// RUN: %clang_dxc -T lib_6_7 -Vd %s -ccc-print-phases 2>&1 | FileCheck %s --check-prefixes=CHECK-PHASES,CHECK-PHASES-ASM
+
+// Same if we explicitly ask for assembly (-Fc) to stdout.
+// RUN: %clang_dxc -T lib_6_7 -Vd %s -Fc - -### 2>&1 | FileCheck %s --check-prefixes=CHECK-CC1,CHECK-CC1-STDOUT
+// RUN: %clang_dxc -T lib_6_7 -Vd %s -Fc - -ccc-print-phases 2>&1 | FileCheck %s --check-prefixes=CHECK-PHASES,CHECK-PHASES-ASM
+
+// DXIL Assembly to a file.
+// RUN: %clang_dxc -T lib_6_7 -Vd %s -Fc x.asm -### 2>&1 | FileCheck %s --check-prefixes=CHECK-CC1,CHECK-CC1-ASM
+// RUN: %clang_dxc -T lib_6_7 -Vd %s -Fcx.asm -### 2>&1 | FileCheck %s --check-prefixes=CHECK-CC1,CHECK-CC1-ASM
+// RUN: %clang_dxc -T lib_6_7 -Vd %s -Fc x.asm -ccc-print-phases 2>&1 | FileCheck %s --check-prefixes=CHECK-PHASES,CHECK-PHASES-ASM
+
+// DXIL Object code to a file.
+// RUN: %clang_dxc -T lib_6_7 -Vd %s -Fo x.obj -### 2>&1 | FileCheck %s --check-prefixes=CHECK-CC1,CHECK-CC1-OBJ
+// RUN: %clang_dxc -T lib_6_7 -Vd %s -Fox.obj -### 2>&1 | FileCheck %s --check-prefixes=CHECK-CC1,CHECK-CC1-OBJ
+// RUN: %clang_dxc -T lib_6_7 -Vd %s -Fo x.obj -ccc-print-phases 2>&1 | FileCheck %s --check-prefixes=CHECK-PHASES,CHECK-PHASES-OBJ
+
+// If both -Fc and -Fo are provided, we generate both files.
+// RUN: %clang_dxc -T lib_6_7 -Vd %s -Fc x.asm -Fo x.obj -### 2>&1 | FileCheck %s --check-prefixes=CHECK-CC1,CHECK-CC1-ASM,CHECK-CC1-BOTH
+// RUN: %clang_dxc -T lib_6_7 -Vd %s -Fc x.asm -Fo x.obj -ccc-print-phases 2>&1 | FileCheck %s --check-prefixes=CHECK-PHASES,CHECK-PHASES-OBJ
+
+// CHECK-PHASES: 0: input, {{.*}}, hlsl
+// CHECK-PHASES: 1: preprocessor, {0}, c++-cpp-output
+// CHECK-PHASES: 2: compiler, {1}, ir
+// CHECK-PHASES: 3: backend, {2}, assembler
+// CHECK-PHASES-ASM-NOT: 4: assembler, {3}, object
+// CHECK-PHASES-OBJ: 4: assembler, {3}, object
+
+// CHECK-CC1: "-cc1"
+// CHECK-CC1-STDOUT-SAME: "-o" "-"
+
+// CHECK-CC1-ASM-SAME: "-S"
+// CHECK-CC1-ASM-SAME: "-o" "x.asm"
+
+// CHECK-CC1-OBJ-SAME: "-emit-obj"
+// CHECK-CC1-OBJ-SAME: "-o" "x.obj"
+
+// For the case where we specify both -Fc and -Fo, we emit the asm as part of
+// cc1 and invoke cc1as for the object.
+// CHECK-CC1-BOTH: "-cc1as"
+// CHECK-CC1-BOTH-SAME: "-o" "x.obj"
Index: clang/test/Driver/dxc_dxv_path.hlsl
===
--- clang/test/Driver/dxc_dxv_path.hlsl
+++ clang/test/Driver/dxc_dxv_path.hlsl
@@ -12,7 +12,7 @@
 
 // RUN: %clang_dxc -Tlib_6_3 -ccc-print-bindings --dxv-path=%T -Fo %t.dxo  %s 2>&1 | FileCheck %s --check-prefix=BINDINGS
 // BINDINGS: "dxil-unknown-shadermodel6.3-library" - "clang", inputs: ["[[INPUT:.+]]"], output: "[[DXC:.+]].dxo"
-// BINDINGS-NEXT: "dxil-unknown-shadermodel6.3-library" - "hlsl::Validator", inputs: ["[[DXC]].dxo"], output: "[[DXC]].dxo"
+// BINDINGS-NEXT: "dxil-unknown-shadermodel6.3-library" - "hlsl::Validator", inputs: ["[[DXC]].dxo"]
 
 // 

[PATCH] D157149: [Option] Add "Visibility" field and clone the OptTable APIs to use it

[Justin Bogner via Phabricator via cfe-commits]

bogner updated this revision to Diff 548832.
bogner added a comment.

Rebase/resolve conflicts


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157149/new/

https://reviews.llvm.org/D157149

Files:
  clang-tools-extra/clangd/CompileCommands.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  lld/MachO/DriverUtils.cpp
  lld/MinGW/Driver.cpp
  lld/wasm/Driver.cpp
  llvm/include/llvm/Option/OptParser.td
  llvm/include/llvm/Option/OptTable.h
  llvm/include/llvm/Option/Option.h
  llvm/lib/ExecutionEngine/JITLink/COFFDirectiveParser.cpp
  llvm/lib/Option/OptTable.cpp
  llvm/lib/ToolDrivers/llvm-dlltool/DlltoolDriver.cpp
  llvm/lib/ToolDrivers/llvm-lib/LibDriver.cpp
  llvm/tools/dsymutil/dsymutil.cpp
  llvm/tools/llvm-cvtres/llvm-cvtres.cpp
  llvm/tools/llvm-cxxfilt/llvm-cxxfilt.cpp
  llvm/tools/llvm-debuginfod/llvm-debuginfod.cpp
  llvm/tools/llvm-dwarfutil/llvm-dwarfutil.cpp
  llvm/tools/llvm-dwp/llvm-dwp.cpp
  llvm/tools/llvm-lipo/llvm-lipo.cpp
  llvm/tools/llvm-mt/llvm-mt.cpp
  llvm/tools/llvm-objcopy/ObjcopyOptions.cpp
  llvm/tools/llvm-rc/llvm-rc.cpp
  llvm/tools/llvm-strings/llvm-strings.cpp
  llvm/tools/llvm-symbolizer/llvm-symbolizer.cpp
  llvm/tools/llvm-tli-checker/llvm-tli-checker.cpp
  llvm/unittests/Option/OptionMarshallingTest.cpp
  llvm/unittests/Option/OptionParsingTest.cpp
  llvm/unittests/Option/Opts.td
  llvm/utils/TableGen/OptParserEmitter.cpp

Index: llvm/utils/TableGen/OptParserEmitter.cpp
===
--- llvm/utils/TableGen/OptParserEmitter.cpp
+++ llvm/utils/TableGen/OptParserEmitter.cpp
@@ -320,7 +320,7 @@
   OS << "INVALID";
 
 // The other option arguments (unused for groups).
-OS << ", INVALID, nullptr, 0, 0";
+OS << ", INVALID, nullptr, 0, 0, 0";
 
 // The option help text.
 if (!isa(R.getValueInit("HelpText"))) {
@@ -358,8 +358,10 @@
 // The containing option group (if any).
 OS << ", ";
 const ListInit *GroupFlags = nullptr;
+const ListInit *GroupVis = nullptr;
 if (const DefInit *DI = dyn_cast(R.getValueInit("Group"))) {
   GroupFlags = DI->getDef()->getValueAsListInit("Flags");
+  GroupVis = DI->getDef()->getValueAsListInit("Vis");
   OS << getOptionName(*DI->getDef());
 } else
   OS << "INVALID";
@@ -400,6 +402,21 @@
 if (NumFlags == 0)
   OS << '0';
 
+// The option visibility flags.
+OS << ", ";
+int NumVisFlags = 0;
+LI = R.getValueAsListInit("Vis");
+for (Init *I : *LI)
+  OS << (NumVisFlags++ ? " | " : "")
+ << cast(I)->getDef()->getName();
+if (GroupVis) {
+  for (Init *I : *GroupVis)
+OS << (NumVisFlags++ ? " | " : "")
+   << cast(I)->getDef()->getName();
+}
+if (NumVisFlags == 0)
+  OS << '0';
+
 // The option parameter field.
 OS << ", " << R.getValueAsInt("NumArgs");
 
Index: llvm/unittests/Option/Opts.td
===
--- llvm/unittests/Option/Opts.td
+++ llvm/unittests/Option/Opts.td
@@ -4,6 +4,8 @@
 def OptFlag2 : OptionFlag;
 def OptFlag3 : OptionFlag;
 
+def SubtoolVis : OptionVisibility;
+
 def A : Flag<["-"], "A">, HelpText<"The A option">, Flags<[OptFlag1]>;
 def AB : Flag<["-"], "AB">;
 def B : Joined<["-"], "B">, HelpText<"The B option">, MetaVarName<"B">, Flags<[OptFlag2]>;
@@ -35,6 +37,8 @@
 def Cramb : Joined<["/"], "cramb:">, HelpText<"The cramb option">, MetaVarName<"CRAMB">, Flags<[OptFlag1]>;
 def Doopf1 : Flag<["-"], "doopf1">, HelpText<"The doopf1 option">, Flags<[OptFlag1]>;
 def Doopf2 : Flag<["-"], "doopf2">, HelpText<"The doopf2 option">, Flags<[OptFlag2]>;
+def Xyzzy1 : Flag<["-"], "xyzzy1">, HelpText<"The xyzzy1 option">, Vis<[SubtoolVis]>;
+def Xyzzy2 : Flag<["-"], "xyzzy2">, HelpText<"The xyzzy2 option">, Vis<[Default]>;
 def Ermgh : Joined<["--"], "ermgh">, HelpText<"The ermgh option">, MetaVarName<"ERMGH">, Flags<[OptFlag1]>;
 def Fjormp : Flag<["--"], "fjormp">, HelpText<"The fjormp option">, Flags<[OptFlag1]>;
 
@@ -43,6 +47,9 @@
 def Blurmpq : Flag<["--"], "blurmp">;
 def Blurmpq_eq : Flag<["--"], "blurmp=">;
 
+def Q : Flag<["-"], "Q">, Vis<[SubtoolVis]>;
+def R : Flag<["-"], "R">, Vis<[Default, SubtoolVis]>;
+
 class XOpts : KeyPathAndMacro<"X->", base> {}
 
 def marshalled_flag_d : Flag<["-"], "marshalled-flag-d">,
Index: llvm/unittests/Option/OptionParsingTest.cpp
===
--- llvm/unittests/Option/OptionParsingTest.cpp
+++ llvm/unittests/Option/OptionParsingTest.cpp
@@ -44,6 +44,10 @@
   OptFlag3 = (1 << 6)
 };
 
+enum OptionVisibility {
+  SubtoolVis = (1 << 2),
+};
+
 static constexpr OptTable::Info InfoTable[] = {
 #define OPTION(...) LLVM_CONSTRUCT_OPT_INFO(__VA_ARGS__),
 #include "Opts.inc"
@@ -163,6 +167,43 @@
   EXPECT_EQ("bar", AL.getLastArgValue(OPT_C));
 }
 
+TYPED_TEST(OptTableTest, ParseWithVisibility) {
+  TypeParam T;
+  unsigned MAI, MAC;
+
+  const char 

[PATCH] D157562: [Driver][DXC] Treat stdin as HLSL

[Justin Bogner via Phabricator via cfe-commits]

bogner created this revision.
bogner added reviewers: beanz, python3kgae.
Herald added subscribers: Anastasia, mcrosier.
Herald added a project: All.
bogner requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay.
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D157562

Files:
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/dxc_stdin.hlsl


Index: clang/test/Driver/dxc_stdin.hlsl
===
--- /dev/null
+++ clang/test/Driver/dxc_stdin.hlsl
@@ -0,0 +1,3 @@
+// RUN: %clang_dxc -Tlib_6_7 - -### 2>&1 | FileCheck %s
+// CHECK: "-cc1"
+// CHECK-SAME: "-x" "hlsl" "-"
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -2648,6 +2648,8 @@
 if (memcmp(Value, "-", 2) == 0) {
   if (IsFlangMode()) {
 Ty = types::TY_Fortran;
+  } else if (IsDXCMode()) {
+Ty = types::TY_HLSL;
   } else {
 // If running with -E, treat as a C input (this changes the
 // builtin macros, for example). This may be overridden by -ObjC


Index: clang/test/Driver/dxc_stdin.hlsl
===
--- /dev/null
+++ clang/test/Driver/dxc_stdin.hlsl
@@ -0,0 +1,3 @@
+// RUN: %clang_dxc -Tlib_6_7 - -### 2>&1 | FileCheck %s
+// CHECK: "-cc1"
+// CHECK-SAME: "-x" "hlsl" "-"
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -2648,6 +2648,8 @@
 if (memcmp(Value, "-", 2) == 0) {
   if (IsFlangMode()) {
 Ty = types::TY_Fortran;
+  } else if (IsDXCMode()) {
+Ty = types::TY_HLSL;
   } else {
 // If running with -E, treat as a C input (this changes the
 // builtin macros, for example). This may be overridden by -ObjC
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D156178: [HLSL] add pow library function

[Justin Bogner via Phabricator via cfe-commits]

bogner added a comment.

In D156178#4530161 , @bogner wrote:

> Looking at 
> https://learn.microsoft.com/en-us/windows/win32/direct3dhlsl/dx-graphics-hlsl-pow,
>  it seems a bit underspecified, but do we need to worry about differences 
> between that spec and IEEE behaviour here? For example, it says there that 
> `x<0, y=any -> NaN`, but we'll only generate `NaN` for non-integer `y` in 
> these cases as per https://en.cppreference.com/w/c/numeric/math/pow

FWIW it looks like the behaviour here matches what DXC does in practice today, 
so discussing the discrepancy here probably doesn't need to block getting this 
in


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156178/new/

https://reviews.llvm.org/D156178

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D157150: [Driver] Update BoolOption to handle Visibility. NFC

[Justin Bogner via Phabricator via cfe-commits]

bogner created this revision.
bogner added a reviewer: MaskRay.
Herald added a subscriber: mcrosier.
Herald added a reviewer: sscalpone.
Herald added a project: All.
bogner requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This updates the BoolOption family of option definitions to do the
right thing with llvm::opt::Visibility. Since the only meaningful
visibility at this point is llvm::opt::Default, we update PosFlag,
NegFlag, and BothFlags definitions to now specify Default.

The updates to option definitions were done with the following three
sed scripts (one to update Pos/NegFlag, one for BothFlags, and one
that just cleans up whitespace a little):

  sed -E 's/((Pos|Neg)Flag<[A-Za-z]*, \[[^]]*\])(, "|>|,$)/\1, [Default]\3/g'
  sed -E 's/(BothFlags<\[[^]]*\])(, ")/\1, [Default], "/'
  sed -E 's/( *)((Pos|Neg)Flag<.*), ((Pos|Neg)Flag)/\1\2,\n\1\4/'

These are idempotent and should be runnable on downstream versions of
Options.td if needed to update any additional flags that are present.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D157150

Files:
  clang/include/clang/Driver/Options.td

Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -381,7 +381,8 @@
 
 // Definition of single command line flag. This is an implementation detail, use
 // SetTrueBy or SetFalseBy instead.
-class FlagDef option_flags,
+class FlagDef option_flags, list option_vis,
   string help, list implied_by_expressions = []> {
   // The polarity. Besides spelling, this also decides whether the TableGen
   // record will be prefixed with "no_".
@@ -390,9 +391,12 @@
   // The value assigned to key path when the flag is present on command line.
   bit Value = value;
 
-  // OptionFlags that control visibility of the flag in different tools.
+  // OptionFlags in different tools.
   list OptionFlags = option_flags;
 
+  // OptionVisibility flags for different tools.
+  list OptionVis = option_vis;
+
   // The help text associated with the flag.
   string Help = help;
 
@@ -401,8 +405,11 @@
 }
 
 // Additional information to be appended to both positive and negative flag.
-class BothFlags option_flags, string help = ""> {
+class BothFlags option_flags,
+list option_vis = [Default],
+string help = ""> {
   list OptionFlags = option_flags;
+  list OptionVis = option_vis;
   string Help = help;
 }
 
@@ -411,23 +418,26 @@
   FlagDef Result
 = FlagDef;
 }
 
 // Definition of the command line flag with positive spelling, e.g. "-ffoo".
-class PosFlag flags = [], string help = "",
-  list implied_by_expressions = []>
-  : FlagDef {}
+class PosFlag flags = [], list vis = [],
+  string help = "", list implied_by_expressions = []>
+  : FlagDef {}
 
 // Definition of the command line flag with negative spelling, e.g. "-fno-foo".
-class NegFlag flags = [], string help = "",
-  list implied_by_expressions = []>
-  : FlagDef {}
+class NegFlag flags = [], list vis = [],
+  string help = "", list implied_by_expressions = []>
+  : FlagDef {}
 
 // Expanded FlagDef that's convenient for creation of TableGen records.
 class FlagDefExpanded
-  : FlagDef {
+  : FlagDef {
   // Name of the TableGen record.
   string RecordName = prefix # !if(flag.Polarity, "", "no_") # name;
 
@@ -445,7 +455,8 @@
 class MarshalledFlagRec
-  : Flag<["-"], flag.Spelling>, Flags, HelpText,
+  : Flag<["-"], flag.Spelling>, Flags, Vis,
+HelpText,
 MarshallingInfoBooleanFlag,
 ImpliedByAnyOf {}
@@ -459,7 +470,7 @@
 multiclass BoolOption> {
+  BothFlags suffix = BothFlags<[]>> {
   defvar flag1 = FlagDefExpanded.Result, prefix,
  NAME, spelling_base>;
 
@@ -490,7 +501,7 @@
 /// CompilerInvocation.
 multiclass BoolFOption> {
+   BothFlags both = BothFlags<[]>> {
   defm NAME : BoolOption<"f", flag_base, kpm, default, flag1, flag2, both>,
   Group;
 }
@@ -501,7 +512,7 @@
 // CompilerInvocation.
 multiclass BoolGOption> {
+   BothFlags both = BothFlags<[]>> {
   defm NAME : BoolOption<"g", flag_base, kpm, default, flag1, flag2, both>,
   Group;
 }
@@ -509,7 +520,7 @@
 // Works like BoolOption except without marshalling
 multiclass BoolOptionWithoutMarshalling> {
+BothFlags suffix = BothFlags<[]>> {
   defvar flag1 = FlagDefExpanded.Result, prefix,
  NAME, spelling_base>;
 
@@ -531,11 +542,11 @@
   defvar implied = !if(flag1.CanBeImplied, flag1, flag2);
 
   def flag1.RecordName : Flag<["-"], flag1.Spelling>, Flags,
- HelpText,
+ Vis, HelpText,
  ImpliedByAnyOf
  {}
   def 

[PATCH] D157149: [Option] Add "Visibility" field and clone the OptTable APIs to use it

[Justin Bogner via Phabricator via cfe-commits]

bogner created this revision.
bogner added a reviewer: MaskRay.
Herald added subscribers: pmatos, asb, ormris, kadircet, arphaman, steven_wu, 
hiraditya, sbc100, mcrosier.
Herald added a reviewer: JDevlieghere.
Herald added a reviewer: alexander-shaposhnikov.
Herald added a reviewer: jhenderson.
Herald added projects: lld-macho, All.
Herald added a reviewer: lld-macho.
bogner requested review of this revision.
Herald added subscribers: cfe-commits, llvm-commits, wangpc, aheejin.
Herald added projects: clang, LLVM, clang-tools-extra.

This splits OptTable's "Flags" field into "Flags" and "Visibility",
updates the places where we instantiate Option tables, and adds
variants of the OptTable APIs that use Visibility mask instead of
Include/Exclude flags.

We need to do this to clean up a bunch of complexity in the clang
driver's option handling - there's a whole slew of flags like
CoreOption, NoDriverOption, and FlangOnlyOption there today to try to
handle all of the permutations of flags that the various drivers need,
but it really doesn't scale well, as can be seen by things like the
somewhat recently introduced CLDXCOption.

Instead, we'll provide an additive model for visibility that's
separate from the other flags, and have the only "subtractive"
visibility flag be "HelpHidden", which can be handled as a special
case.

Note that we don't actually update the users of the Include/Exclude
APIs here or change the flags that exist in clang at all - that will
come in a follow up that refactors clang's Options.td to use the
increased flexibility this change allows.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D157149

Files:
  clang-tools-extra/clangd/CompileCommands.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  lld/MachO/DriverUtils.cpp
  lld/MinGW/Driver.cpp
  lld/wasm/Driver.cpp
  llvm/include/llvm/Option/OptParser.td
  llvm/include/llvm/Option/OptTable.h
  llvm/include/llvm/Option/Option.h
  llvm/lib/ExecutionEngine/JITLink/COFFDirectiveParser.cpp
  llvm/lib/Option/OptTable.cpp
  llvm/lib/ToolDrivers/llvm-dlltool/DlltoolDriver.cpp
  llvm/lib/ToolDrivers/llvm-lib/LibDriver.cpp
  llvm/tools/dsymutil/dsymutil.cpp
  llvm/tools/llvm-cvtres/llvm-cvtres.cpp
  llvm/tools/llvm-cxxfilt/llvm-cxxfilt.cpp
  llvm/tools/llvm-debuginfod/llvm-debuginfod.cpp
  llvm/tools/llvm-dwarfutil/llvm-dwarfutil.cpp
  llvm/tools/llvm-dwp/llvm-dwp.cpp
  llvm/tools/llvm-lipo/llvm-lipo.cpp
  llvm/tools/llvm-mt/llvm-mt.cpp
  llvm/tools/llvm-objcopy/ObjcopyOptions.cpp
  llvm/tools/llvm-rc/llvm-rc.cpp
  llvm/tools/llvm-strings/llvm-strings.cpp
  llvm/tools/llvm-symbolizer/llvm-symbolizer.cpp
  llvm/tools/llvm-tli-checker/llvm-tli-checker.cpp
  llvm/unittests/Option/OptionMarshallingTest.cpp
  llvm/unittests/Option/OptionParsingTest.cpp
  llvm/unittests/Option/Opts.td
  llvm/utils/TableGen/OptParserEmitter.cpp

Index: llvm/utils/TableGen/OptParserEmitter.cpp
===
--- llvm/utils/TableGen/OptParserEmitter.cpp
+++ llvm/utils/TableGen/OptParserEmitter.cpp
@@ -320,7 +320,7 @@
   OS << "INVALID";
 
 // The other option arguments (unused for groups).
-OS << ", INVALID, nullptr, 0, 0";
+OS << ", INVALID, nullptr, 0, 0, 0";
 
 // The option help text.
 if (!isa(R.getValueInit("HelpText"))) {
@@ -358,8 +358,10 @@
 // The containing option group (if any).
 OS << ", ";
 const ListInit *GroupFlags = nullptr;
+const ListInit *GroupVis = nullptr;
 if (const DefInit *DI = dyn_cast(R.getValueInit("Group"))) {
   GroupFlags = DI->getDef()->getValueAsListInit("Flags");
+  GroupVis = DI->getDef()->getValueAsListInit("Vis");
   OS << getOptionName(*DI->getDef());
 } else
   OS << "INVALID";
@@ -400,6 +402,21 @@
 if (NumFlags == 0)
   OS << '0';
 
+// The option visibility flags.
+OS << ", ";
+int NumVisFlags = 0;
+LI = R.getValueAsListInit("Vis");
+for (Init *I : *LI)
+  OS << (NumVisFlags++ ? " | " : "")
+ << cast(I)->getDef()->getName();
+if (GroupVis) {
+  for (Init *I : *GroupVis)
+OS << (NumVisFlags++ ? " | " : "")
+   << cast(I)->getDef()->getName();
+}
+if (NumVisFlags == 0)
+  OS << '0';
+
 // The option parameter field.
 OS << ", " << R.getValueAsInt("NumArgs");
 
Index: llvm/unittests/Option/Opts.td
===
--- llvm/unittests/Option/Opts.td
+++ llvm/unittests/Option/Opts.td
@@ -4,6 +4,8 @@
 def OptFlag2 : OptionFlag;
 def OptFlag3 : OptionFlag;
 
+def SubtoolVis : OptionVisibility;
+
 def A : Flag<["-"], "A">, HelpText<"The A option">, Flags<[OptFlag1]>;
 def AB : Flag<["-"], "AB">;
 def B : Joined<["-"], "B">, HelpText<"The B option">, MetaVarName<"B">, Flags<[OptFlag2]>;
@@ -35,6 +37,8 @@
 def Cramb : Joined<["/"], "cramb:">, HelpText<"The cramb option">, MetaVarName<"CRAMB">, Flags<[OptFlag1]>;
 def Doopf1 : Flag<["-"], "doopf1">, HelpText<"The doopf1 option">, 

[PATCH] D157146: [Clang][Docs] Consolidate option hiding in ClangOptionDocEmitter

[Justin Bogner via Phabricator via cfe-commits]

bogner created this revision.
bogner added a reviewer: MaskRay.
Herald added a subscriber: mcrosier.
Herald added a project: All.
bogner requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Update the `hasFlag` check to account for an Option's groups to better
match how the option parsing logic works, and instead of checking if a
group has include/exclude flags just check if there are any visible
options in it.

This cleans up some the empty sections that are currently emitted in
clang's option docs.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D157146

Files:
  clang/utils/TableGen/ClangOptionDocEmitter.cpp

Index: clang/utils/TableGen/ClangOptionDocEmitter.cpp
===
--- clang/utils/TableGen/ClangOptionDocEmitter.cpp
+++ clang/utils/TableGen/ClangOptionDocEmitter.cpp
@@ -31,13 +31,41 @@
 struct Documentation {
   std::vector Groups;
   std::vector Options;
+
+  bool empty() {
+return Groups.empty() && Options.empty();
+  }
 };
 struct DocumentedGroup : Documentation {
   Record *Group;
 };
 
+bool hasFlag(const Record *Option, StringRef OptionFlag) {
+  for (const Record *Flag : Option->getValueAsListOfDefs("Flags"))
+if (Flag->getName() == OptionFlag)
+  return true;
+  if (const DefInit *DI = dyn_cast(Option->getValueInit("Group")))
+for (const Record *Flag : DI->getDef()->getValueAsListOfDefs("Flags"))
+  if (Flag->getName() == OptionFlag)
+return true;
+  return false;
+}
+
+bool isOptionVisible(const Record *Option, const Record *DocInfo) {
+  for (StringRef Exclusion : DocInfo->getValueAsListOfStrings("ExcludedFlags"))
+if (hasFlag(Option, Exclusion))
+  return false;
+  if (!DocInfo->getValue("IncludedFlags"))
+return true;
+  for (StringRef Inclusion : DocInfo->getValueAsListOfStrings("IncludedFlags"))
+if (hasFlag(Option, Inclusion))
+  return true;
+  return false;
+}
+
 // Reorganize the records into a suitable form for emitting documentation.
-Documentation extractDocumentation(RecordKeeper ) {
+Documentation extractDocumentation(RecordKeeper ,
+   const Record *DocInfo) {
   Documentation Result;
 
   // Build the tree of groups. The root in the tree is the fake option group
@@ -124,12 +152,15 @@
   D.Groups.back().Group = G;
   Documentation  = D.Groups.back();
   Base = DocumentationForGroup(G);
+  if (Base.empty())
+D.Groups.pop_back();
 }
 
 auto  = OptionsInGroup[R];
 llvm::sort(Options, CompareByName);
 for (Record *O : Options)
-  D.Options.push_back(DocumentationForOption(O));
+  if (isOptionVisible(O, DocInfo))
+D.Options.push_back(DocumentationForOption(O));
 
 return D;
   };
@@ -161,44 +192,6 @@
 .Default(0);
 }
 
-bool hasFlag(const Record *OptionOrGroup, StringRef OptionFlag) {
-  for (const Record *Flag : OptionOrGroup->getValueAsListOfDefs("Flags"))
-if (Flag->getName() == OptionFlag)
-  return true;
-  return false;
-}
-
-bool isIncluded(const Record *OptionOrGroup, const Record *DocInfo) {
-  assert(DocInfo->getValue("IncludedFlags") && "Missing includeFlags");
-  for (StringRef Inclusion : DocInfo->getValueAsListOfStrings("IncludedFlags"))
-if (hasFlag(OptionOrGroup, Inclusion))
-  return true;
-  return false;
-}
-
-bool isGroupIncluded(const DocumentedGroup , const Record *DocInfo) {
-  if (isIncluded(Group.Group, DocInfo))
-return true;
-  for (auto  : Group.Options)
-if (isIncluded(O.Option, DocInfo))
-  return true;
-  for (auto  : Group.Groups) {
-if (isIncluded(G.Group, DocInfo))
-  return true;
-if (isGroupIncluded(G, DocInfo))
-  return true;
-  }
-  return false;
-}
-
-bool isExcluded(const Record *OptionOrGroup, const Record *DocInfo) {
-  // FIXME: Provide a flag to specify the set of exclusions.
-  for (StringRef Exclusion : DocInfo->getValueAsListOfStrings("ExcludedFlags"))
-if (hasFlag(OptionOrGroup, Exclusion))
-  return true;
-  return false;
-}
-
 std::string escapeRST(StringRef Str) {
   std::string Out;
   for (auto K : Str) {
@@ -319,16 +312,13 @@
   F(Option.Option);
 
   for (auto *Alias : Option.Aliases)
-if (!isExcluded(Alias, DocInfo) && canSphinxCopeWithOption(Option.Option))
+if (isOptionVisible(Alias, DocInfo) &&
+canSphinxCopeWithOption(Option.Option))
   F(Alias);
 }
 
 void emitOption(const DocumentedOption , const Record *DocInfo,
 raw_ostream ) {
-  if (isExcluded(Option.Option, DocInfo))
-return;
-  if (DocInfo->getValue("IncludedFlags") && !isIncluded(Option.Option, DocInfo))
-return;
   if (Option.Option->getValueAsDef("Kind")->getName() == "KIND_UNKNOWN" ||
   Option.Option->getValueAsDef("Kind")->getName() == "KIND_INPUT")
 return;
@@ -401,12 +391,6 @@
 
 void emitGroup(int Depth, const DocumentedGroup , const Record *DocInfo,
raw_ostream ) 

[PATCH] D156925: [Driver] Don't try to spell check unsupported options

[Justin Bogner via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG7ef1718c4d4e: [Driver] Dont try to spell check 
unsupported options (authored by bogner).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156925/new/

https://reviews.llvm.org/D156925

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/unsupported-option.c
  flang/test/Driver/driver-version.f90


Index: flang/test/Driver/driver-version.f90
===
--- flang/test/Driver/driver-version.f90
+++ flang/test/Driver/driver-version.f90
@@ -9,7 +9,7 @@
 ! VERSION-NEXT: Thread model:
 ! VERSION-NEXT: InstalledDir:
 
-! ERROR: flang-new: error: unsupported option '--versions'; did you mean 
'--version'?
+! ERROR: flang-new: error: unknown argument '--versions'; did you mean 
'--version'?
 
 ! VERSION-FC1: LLVM version
 
Index: clang/test/Driver/unsupported-option.c
===
--- clang/test/Driver/unsupported-option.c
+++ clang/test/Driver/unsupported-option.c
@@ -1,10 +1,10 @@
 // RUN: not %clang %s --hedonism -### 2>&1 | \
 // RUN: FileCheck %s
-// CHECK: error: unsupported option '--hedonism'
+// CHECK: error: unknown argument: '--hedonism'
 
 // RUN: not %clang %s --hell -### 2>&1 | \
 // RUN: FileCheck %s --check-prefix=DID-YOU-MEAN
-// DID-YOU-MEAN: error: unsupported option '--hell'; did you mean '--help'?
+// DID-YOU-MEAN: error: unknown argument '--hell'; did you mean '--help'?
 
 // RUN: not %clang --target=powerpc-ibm-aix %s -mlong-double-128 2>&1 | \
 // RUN: FileCheck %s --check-prefix=AIX-LONGDOUBLE128-ERR
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -286,19 +286,9 @@
   // Check for unsupported options.
   for (const Arg *A : Args) {
 if (A->getOption().hasFlag(options::Unsupported)) {
-  unsigned DiagID;
-  auto ArgString = A->getAsString(Args);
-  std::string Nearest;
-  if (getOpts().findNearest(
-ArgString, Nearest, IncludedFlagsBitmask,
-ExcludedFlagsBitmask | options::Unsupported) > 1) {
-DiagID = diag::err_drv_unsupported_opt;
-Diag(DiagID) << ArgString;
-  } else {
-DiagID = diag::err_drv_unsupported_opt_with_suggestion;
-Diag(DiagID) << ArgString << Nearest;
-  }
-  ContainsError |= Diags.getDiagnosticLevel(DiagID, SourceLocation()) >
+  Diag(diag::err_drv_unsupported_opt) << A->getAsString(Args);
+  ContainsError |= Diags.getDiagnosticLevel(diag::err_drv_unsupported_opt,
+SourceLocation()) >
DiagnosticsEngine::Warning;
   continue;
 }
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -4773,7 +4773,6 @@
 def _warn_ : Joined<["--"], "warn-">, Alias;
 def _write_dependencies : Flag<["--"], "write-dependencies">, Alias;
 def _write_user_dependencies : Flag<["--"], "write-user-dependencies">, 
Alias;
-def _ : Joined<["--"], "">, Flags<[Unsupported]>;
 
 // Hexagon feature flags.
 let Flags = [TargetSpecific] in {


Index: flang/test/Driver/driver-version.f90
===
--- flang/test/Driver/driver-version.f90
+++ flang/test/Driver/driver-version.f90
@@ -9,7 +9,7 @@
 ! VERSION-NEXT: Thread model:
 ! VERSION-NEXT: InstalledDir:
 
-! ERROR: flang-new: error: unsupported option '--versions'; did you mean '--version'?
+! ERROR: flang-new: error: unknown argument '--versions'; did you mean '--version'?
 
 ! VERSION-FC1: LLVM version
 
Index: clang/test/Driver/unsupported-option.c
===
--- clang/test/Driver/unsupported-option.c
+++ clang/test/Driver/unsupported-option.c
@@ -1,10 +1,10 @@
 // RUN: not %clang %s --hedonism -### 2>&1 | \
 // RUN: FileCheck %s
-// CHECK: error: unsupported option '--hedonism'
+// CHECK: error: unknown argument: '--hedonism'
 
 // RUN: not %clang %s --hell -### 2>&1 | \
 // RUN: FileCheck %s --check-prefix=DID-YOU-MEAN
-// DID-YOU-MEAN: error: unsupported option '--hell'; did you mean '--help'?
+// DID-YOU-MEAN: error: unknown argument '--hell'; did you mean '--help'?
 
 // RUN: not %clang --target=powerpc-ibm-aix %s -mlong-double-128 2>&1 | \
 // RUN: FileCheck %s --check-prefix=AIX-LONGDOUBLE128-ERR
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -286,19 +286,9 @@
   // Check for unsupported options.
   for (const Arg *A : Args) {
 if 

[PATCH] D156925: [Driver] Don't try to spell check unsupported options

[Justin Bogner via Phabricator via cfe-commits]

bogner created this revision.
bogner added reviewers: Bigcheese, MaskRay.
Herald added a subscriber: mcrosier.
Herald added a reviewer: sscalpone.
Herald added a reviewer: awarzynski.
Herald added projects: Flang, All.
bogner requested review of this revision.
Herald added subscribers: cfe-commits, wangpc, jdoerfert.
Herald added a project: clang.

We currently spell check options that are listed as unsupported, but
this doesn't make much sense. If an option is explicitly unsupported
why would one that's spelled similarly be useful?

It looks like the reason this was added was that we explicitly mark
all `--something` flags as Unsupported rather than just leaving them
undefined and treating them as unknown. Drop that handling so that we
don't regress on things like misspelling `--help`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D156925

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/unsupported-option.c
  flang/test/Driver/driver-version.f90


Index: flang/test/Driver/driver-version.f90
===
--- flang/test/Driver/driver-version.f90
+++ flang/test/Driver/driver-version.f90
@@ -9,7 +9,7 @@
 ! VERSION-NEXT: Thread model:
 ! VERSION-NEXT: InstalledDir:
 
-! ERROR: flang-new: error: unsupported option '--versions'; did you mean 
'--version'?
+! ERROR: flang-new: error: unknown argument '--versions'; did you mean 
'--version'?
 
 ! VERSION-FC1: LLVM version
 
Index: clang/test/Driver/unsupported-option.c
===
--- clang/test/Driver/unsupported-option.c
+++ clang/test/Driver/unsupported-option.c
@@ -1,10 +1,10 @@
 // RUN: not %clang %s --hedonism -### 2>&1 | \
 // RUN: FileCheck %s
-// CHECK: error: unsupported option '--hedonism'
+// CHECK: error: unknown argument: '--hedonism'
 
 // RUN: not %clang %s --hell -### 2>&1 | \
 // RUN: FileCheck %s --check-prefix=DID-YOU-MEAN
-// DID-YOU-MEAN: error: unsupported option '--hell'; did you mean '--help'?
+// DID-YOU-MEAN: error: unknown argument '--hell'; did you mean '--help'?
 
 // RUN: not %clang --target=powerpc-ibm-aix %s -mlong-double-128 2>&1 | \
 // RUN: FileCheck %s --check-prefix=AIX-LONGDOUBLE128-ERR
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -286,19 +286,9 @@
   // Check for unsupported options.
   for (const Arg *A : Args) {
 if (A->getOption().hasFlag(options::Unsupported)) {
-  unsigned DiagID;
-  auto ArgString = A->getAsString(Args);
-  std::string Nearest;
-  if (getOpts().findNearest(
-ArgString, Nearest, IncludedFlagsBitmask,
-ExcludedFlagsBitmask | options::Unsupported) > 1) {
-DiagID = diag::err_drv_unsupported_opt;
-Diag(DiagID) << ArgString;
-  } else {
-DiagID = diag::err_drv_unsupported_opt_with_suggestion;
-Diag(DiagID) << ArgString << Nearest;
-  }
-  ContainsError |= Diags.getDiagnosticLevel(DiagID, SourceLocation()) >
+  Diag(diag::err_drv_unsupported_opt) << A->getAsString(Args);
+  ContainsError |= Diags.getDiagnosticLevel(diag::err_drv_unsupported_opt,
+SourceLocation()) >
DiagnosticsEngine::Warning;
   continue;
 }
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -4773,7 +4773,6 @@
 def _warn_ : Joined<["--"], "warn-">, Alias;
 def _write_dependencies : Flag<["--"], "write-dependencies">, Alias;
 def _write_user_dependencies : Flag<["--"], "write-user-dependencies">, 
Alias;
-def _ : Joined<["--"], "">, Flags<[Unsupported]>;
 
 // Hexagon feature flags.
 let Flags = [TargetSpecific] in {


Index: flang/test/Driver/driver-version.f90
===
--- flang/test/Driver/driver-version.f90
+++ flang/test/Driver/driver-version.f90
@@ -9,7 +9,7 @@
 ! VERSION-NEXT: Thread model:
 ! VERSION-NEXT: InstalledDir:
 
-! ERROR: flang-new: error: unsupported option '--versions'; did you mean '--version'?
+! ERROR: flang-new: error: unknown argument '--versions'; did you mean '--version'?
 
 ! VERSION-FC1: LLVM version
 
Index: clang/test/Driver/unsupported-option.c
===
--- clang/test/Driver/unsupported-option.c
+++ clang/test/Driver/unsupported-option.c
@@ -1,10 +1,10 @@
 // RUN: not %clang %s --hedonism -### 2>&1 | \
 // RUN: FileCheck %s
-// CHECK: error: unsupported option '--hedonism'
+// CHECK: error: unknown argument: '--hedonism'
 
 // RUN: not %clang %s --hell -### 2>&1 | \
 // RUN: FileCheck %s --check-prefix=DID-YOU-MEAN
-// DID-YOU-MEAN: error: unsupported option '--hell'; did you 

[PATCH] D156178: [HLSL] add pow library function

[Justin Bogner via Phabricator via cfe-commits]

bogner added a comment.

Looking at 
https://learn.microsoft.com/en-us/windows/win32/direct3dhlsl/dx-graphics-hlsl-pow,
 it seems a bit underspecified, but do we need to worry about differences 
between that spec and IEEE behaviour here? For example, it says there that 
`x<0, y=any -> NaN`, but we'll only generate `NaN` for non-integer `y` in these 
cases as per https://en.cppreference.com/w/c/numeric/math/pow




Comment at: clang/test/CodeGenHLSL/builtins/pow.hlsl:10
+// CHECK: call half @llvm.pow.f16(
+// NO_HALF: define noundef float @"?test_pow_half@@YA$halff@$halff@0@Z"(
+// NO_HALF: call float @llvm.pow.f32(

Why check the function name in the no-half case but not the other check?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156178/new/

https://reviews.llvm.org/D156178

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D155729: [OptTable] Make explicitly included options override excluded ones

[Justin Bogner via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGb2eda85f047f: [OptTable] Make explicitly included options 
override excluded ones (authored by bogner).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D155729/new/

https://reviews.llvm.org/D155729

Files:
  clang-tools-extra/clangd/CompileCommands.cpp
  clang/include/clang/Driver/Options.h
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/Driver.cpp
  clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
  llvm/lib/Option/OptTable.cpp

Index: llvm/lib/Option/OptTable.cpp
===
--- llvm/lib/Option/OptTable.cpp
+++ llvm/lib/Option/OptTable.cpp
@@ -421,7 +421,8 @@
 if (FlagsToInclude && !Opt.hasFlag(FlagsToInclude))
   continue;
 if (Opt.hasFlag(FlagsToExclude))
-  continue;
+  if (!FlagsToInclude || !Opt.hasFlag(FlagsToInclude))
+continue;
 
 // See if this option matches.
 if (std::unique_ptr A =
@@ -650,7 +651,8 @@
 if (FlagsToInclude && !(Flags & FlagsToInclude))
   continue;
 if (Flags & FlagsToExclude)
-  continue;
+  if (!FlagsToInclude || !(Flags & FlagsToInclude))
+continue;
 
 // If an alias doesn't have a help text, show a help text for the aliased
 // option instead.
Index: clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
===
--- clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
+++ clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
@@ -164,8 +164,8 @@
   const unsigned OldPos = Pos;
   std::unique_ptr Arg(OptTable.ParseOneArg(
   ArgList, Pos,
-  /* Include */ ClangCLMode ? CoreOption | CLOption | CLDXCOption : 0,
-  /* Exclude */ ClangCLMode ? 0 : CLOption | CLDXCOption));
+  /* Include */ ClangCLMode ? CoreOption | CLOption : 0,
+  /* Exclude */ ClangCLMode ? 0 : CLOption));
 
   if (!Arg)
 continue;
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -6496,7 +6496,6 @@
   if (IsClCompatMode) {
 // Include CL and Core options.
 IncludedFlagsBitmask |= options::CLOption;
-IncludedFlagsBitmask |= options::CLDXCOption;
 IncludedFlagsBitmask |= options::CoreOption;
   } else {
 ExcludedFlagsBitmask |= options::CLOption;
@@ -6504,13 +6503,10 @@
   if (IsDXCMode()) {
 // Include DXC and Core options.
 IncludedFlagsBitmask |= options::DXCOption;
-IncludedFlagsBitmask |= options::CLDXCOption;
 IncludedFlagsBitmask |= options::CoreOption;
   } else {
 ExcludedFlagsBitmask |= options::DXCOption;
   }
-  if (!IsClCompatMode && !IsDXCMode())
-ExcludedFlagsBitmask |= options::CLDXCOption;
 
   return std::make_pair(IncludedFlagsBitmask, ExcludedFlagsBitmask);
 }
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -53,10 +53,6 @@
 // are made available when the driver is running in DXC compatibility mode.
 def DXCOption : OptionFlag;
 
-// CLDXCOption - This is a cl.exe/dxc.exe compatibility option. Options with this flag
-// are made available when the driver is running in CL/DXC compatibility mode.
-def CLDXCOption : OptionFlag;
-
 // NoDriverOption - This option should not be accepted by the driver.
 def NoDriverOption : OptionFlag;
 
@@ -6856,7 +6852,7 @@
 // clang-cl Options
 //===--===//
 
-def cl_Group : OptionGroup<"">, Flags<[CLDXCOption]>,
+def cl_Group : OptionGroup<"">,
   HelpText<"CL.EXE COMPATIBILITY OPTIONS">;
 
 def cl_compile_Group : OptionGroup<"">,
@@ -6865,42 +6861,45 @@
 def cl_ignored_Group : OptionGroup<"">,
   Group;
 
-class CLFlag : Option<["/", "-"], name, KIND_FLAG>,
-  Group, Flags<[CLOption, NoXarchOption]>;
-
-class CLDXCFlag : Option<["/", "-"], name, KIND_FLAG>,
-  Group, Flags<[CLDXCOption, NoXarchOption]>;
-
-class CLCompileFlag : Option<["/", "-"], name, KIND_FLAG>,
-  Group, Flags<[CLOption, NoXarchOption]>;
+class CLFlagOpts flags>
+  : Flags;
 
-class CLIgnoredFlag : Option<["/", "-"], name, KIND_FLAG>,
-  Group, Flags<[CLOption, NoXarchOption]>;
+class CLFlag flags = [CLOption]>
+  : Option<["/", "-"], name, KIND_FLAG>,
+Group, CLFlagOpts;
 
-class CLJoined : Option<["/", "-"], name, KIND_JOINED>,
-  Group, Flags<[CLOption, NoXarchOption]>;
+class CLCompileFlag flags = [CLOption]>
+  : Option<["/", "-"], name, KIND_FLAG>,
+Group, CLFlagOpts;
 
-class CLDXCJoined : Option<["/", "-"], name, KIND_JOINED>,
-  Group, Flags<[CLDXCOption, NoXarchOption]>;
+class CLIgnoredFlag flags = [CLOption]>
+  : Option<["/", "-"], name, 

[PATCH] D155729: [OptTable] Make explicitly included options override excluded ones

[Justin Bogner via Phabricator via cfe-commits]

bogner added inline comments.



Comment at: clang/include/clang/Driver/Options.h:40
+  Ignored = (1 << 18),
+  TargetSpecific = (1 << 19),
 };

bob80905 wrote:
> Given that the id for these flags have changed, does it make sense to write a 
> test that makes sure these flags with these bits still behave as expected? 
> (Ignored and target specific, specifically).
The actual values of the flags aren't really visible anywhere (and in the case 
of ignored, it's actually just changing the value back to what it was before 
https://reviews.llvm.org/D128462). I'm not sure that there's much we can do to 
write such a test.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D155729/new/

https://reviews.llvm.org/D155729

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D155729: [OptTable] Make explicitly included options override excluded ones

[Justin Bogner via Phabricator via cfe-commits]

bogner created this revision.
bogner added reviewers: beanz, kadircet, python3kgae, hans.
Herald added subscribers: arphaman, hiraditya, mcrosier.
Herald added a project: All.
bogner requested review of this revision.
Herald added subscribers: cfe-commits, llvm-commits, MaskRay.
Herald added projects: clang, LLVM, clang-tools-extra.

When we have both explicitly included and excluded option sets, we
were excluding anything from the latter set regardless of what was in
the former. This doesn't compose well and led to an overly complicated
design around DXC options where a third flag was introduced to handle
options that overlapped between DXC and CL.

With this change we check the included options before excluding
anything from the exclude list, which allows for options that are in
multiple categories to be handled in a sensible way. This allows us to
remove CLDXCOption but should otherwise be NFC.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D155729

Files:
  clang-tools-extra/clangd/CompileCommands.cpp
  clang/include/clang/Driver/Options.h
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/Driver.cpp
  clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
  llvm/lib/Option/OptTable.cpp

Index: llvm/lib/Option/OptTable.cpp
===
--- llvm/lib/Option/OptTable.cpp
+++ llvm/lib/Option/OptTable.cpp
@@ -421,7 +421,8 @@
 if (FlagsToInclude && !Opt.hasFlag(FlagsToInclude))
   continue;
 if (Opt.hasFlag(FlagsToExclude))
-  continue;
+  if (!FlagsToInclude || !Opt.hasFlag(FlagsToInclude))
+continue;
 
 // See if this option matches.
 if (std::unique_ptr A =
@@ -650,7 +651,8 @@
 if (FlagsToInclude && !(Flags & FlagsToInclude))
   continue;
 if (Flags & FlagsToExclude)
-  continue;
+  if (!FlagsToInclude || !(Flags & FlagsToInclude))
+continue;
 
 // If an alias doesn't have a help text, show a help text for the aliased
 // option instead.
Index: clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
===
--- clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
+++ clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
@@ -164,8 +164,8 @@
   const unsigned OldPos = Pos;
   std::unique_ptr Arg(OptTable.ParseOneArg(
   ArgList, Pos,
-  /* Include */ ClangCLMode ? CoreOption | CLOption | CLDXCOption : 0,
-  /* Exclude */ ClangCLMode ? 0 : CLOption | CLDXCOption));
+  /* Include */ ClangCLMode ? CoreOption | CLOption : 0,
+  /* Exclude */ ClangCLMode ? 0 : CLOption));
 
   if (!Arg)
 continue;
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -6482,7 +6482,6 @@
   if (IsClCompatMode) {
 // Include CL and Core options.
 IncludedFlagsBitmask |= options::CLOption;
-IncludedFlagsBitmask |= options::CLDXCOption;
 IncludedFlagsBitmask |= options::CoreOption;
   } else {
 ExcludedFlagsBitmask |= options::CLOption;
@@ -6490,13 +6489,10 @@
   if (IsDXCMode()) {
 // Include DXC and Core options.
 IncludedFlagsBitmask |= options::DXCOption;
-IncludedFlagsBitmask |= options::CLDXCOption;
 IncludedFlagsBitmask |= options::CoreOption;
   } else {
 ExcludedFlagsBitmask |= options::DXCOption;
   }
-  if (!IsClCompatMode && !IsDXCMode())
-ExcludedFlagsBitmask |= options::CLDXCOption;
 
   return std::make_pair(IncludedFlagsBitmask, ExcludedFlagsBitmask);
 }
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -53,10 +53,6 @@
 // are made available when the driver is running in DXC compatibility mode.
 def DXCOption : OptionFlag;
 
-// CLDXCOption - This is a cl.exe/dxc.exe compatibility option. Options with this flag
-// are made available when the driver is running in CL/DXC compatibility mode.
-def CLDXCOption : OptionFlag;
-
 // NoDriverOption - This option should not be accepted by the driver.
 def NoDriverOption : OptionFlag;
 
@@ -6844,7 +6840,7 @@
 // clang-cl Options
 //===--===//
 
-def cl_Group : OptionGroup<"">, Flags<[CLDXCOption]>,
+def cl_Group : OptionGroup<"">,
   HelpText<"CL.EXE COMPATIBILITY OPTIONS">;
 
 def cl_compile_Group : OptionGroup<"">,
@@ -6853,42 +6849,45 @@
 def cl_ignored_Group : OptionGroup<"">,
   Group;
 
-class CLFlag : Option<["/", "-"], name, KIND_FLAG>,
-  Group, Flags<[CLOption, NoXarchOption]>;
-
-class CLDXCFlag : Option<["/", "-"], name, KIND_FLAG>,
-  Group, Flags<[CLDXCOption, NoXarchOption]>;
-
-class CLCompileFlag : Option<["/", "-"], name, KIND_FLAG>,
-  Group, Flags<[CLOption, NoXarchOption]>;
+class CLFlagOpts flags>
+  : Flags;
 

[PATCH] D142473: [UTC] Add --version argument

[Justin Bogner via Phabricator via cfe-commits]

bogner added a comment.

Multiple people had concerns with the added complexity of having to maintain 
historical versions of behaviour here (a concern which I share), and absolutely 
none of them accepted the revision or commented after the comment that you 
don’t think it’ll be too bad in practice (with little explanation as to why).

I really don’t think it was appropriate to commit this without further 
discussion here.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D142473/new/

https://reviews.llvm.org/D142473

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D138571: Update ninja and cmake installation commands on LibASTMatchersTutorial

[Justin Bogner via Phabricator via cfe-commits]

bogner accepted this revision.
bogner added a comment.
This revision is now accepted and ready to land.

lgtm


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D138571/new/

https://reviews.llvm.org/D138571

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D135595: [HLSL] Add utility to convert environment to stage

[Justin Bogner via Phabricator via cfe-commits]

bogner accepted this revision.
bogner added a comment.
This revision is now accepted and ready to land.

Even without the tooling aspect the addition of the static asserts is a strict 
improvement. looks good, thanks!




Comment at: clang/include/clang/Basic/HLSLRuntime.h:39
+  return static_cast(Pipeline);
+}
+

You're not actually introducing the dependency here (it was already there), but 
neither `ShaderStage` in LangOptions.h nor the shader stage `EnvironmentType` 
in Triple.h mention that they need to be kept in sync with the other. Can you 
add comments to both that note the relationship?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D135595/new/

https://reviews.llvm.org/D135595

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131718: [HLSL] Add abs library function

[Justin Bogner via Phabricator via cfe-commits]

bogner accepted this revision.
bogner added a comment.
This revision is now accepted and ready to land.

LGTM. Make sure you update the description from saying that this doesn't handle 
half because of https://llvm.org/pr57100 to saying that it fixes the bug 
instead.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131718/new/

https://reviews.llvm.org/D131718

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D130017: [HLSL] Add RWBuffer default constructor

[Justin Bogner via Phabricator via cfe-commits]

bogner accepted this revision.
bogner added a comment.
This revision is now accepted and ready to land.

Is it worth adding a test that calls this constructor / checks that we end up 
with a call to the builtin?




Comment at: clang/lib/Sema/HLSLExternalSemaSource.cpp:155
+AST, Fn,
+{emitResourceClassExpr(AST, RC)},
+AST.VoidPtrTy, VK_PRValue, SourceLocation(), FPOptionsOverride());

In this case it just creates a literal so it's technically fine, but in general 
seeing these "emit" functions in argument lists can lead to things like the 
generated IR being in a different order depending on the compiler. Better to 
store the Expr in a variable as a matter of style.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130017/new/

https://reviews.llvm.org/D130017

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D128569: Start support for HLSL `RWBuffer`

[Justin Bogner via Phabricator via cfe-commits]

bogner accepted this revision.
bogner added a comment.
This revision is now accepted and ready to land.

Fairly straightforward. LGTM with a couple of nits




Comment at: clang/include/clang/Sema/HLSLExternalSemaSource.h:48
+
+  // Complete an incomplete HLSL builtin type
+  void CompleteType(TagDecl *Tag) override;

This should be "///" for doxygen



Comment at: clang/test/AST/HLSL/RWBuffer-AST.hlsl:2
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-library -x hlsl 
-fsyntax-only -ast-dump -DEMPTY %s | FileCheck -check-prefix=EMPTY %s 
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-library -x hlsl 
-fsyntax-only -ast-dump %s | FileCheck %s 
+

Might be worth putting a comment at the top of this test explaining the 
difference between the two test modes here


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D128569/new/

https://reviews.llvm.org/D128569

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D124753: [HLSL] Set main as default entry.

[Justin Bogner via Phabricator via cfe-commits]

bogner added inline comments.



Comment at: clang/test/CodeGenHLSL/entry_default.hlsl:14
+// CHECK-NOT: "dx.shader"="compute"
+// CHECK-SAM: }
+[numthreads(1, 1, 1)] void main() {

typo, should be `CHECK-SAME`


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D124753/new/

https://reviews.llvm.org/D124753

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D123907: [HLSL] Add shader attribute

[Justin Bogner via Phabricator via cfe-commits]

bogner added inline comments.



Comment at: clang/include/clang/Basic/AttrDocs.td:6390
+  ``[shader(string-literal)]``
+where the string literal is one of: .
+Normally the shader type is set by shader target with the ``-T`` option like

I'm guessing you meant to actually list the options here?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123907/new/

https://reviews.llvm.org/D123907

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D94533: [clang] Add AddClang.cmake to the list of the CMake modules that are installed

[Justin Bogner via Phabricator via cfe-commits]

bogner accepted this revision.
bogner added a comment.
This revision is now accepted and ready to land.

This seems reasonable


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94533/new/

https://reviews.llvm.org/D94533

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D62693: Support codesigning bundles and forcing

[Justin Bogner via Phabricator via cfe-commits]

bogner accepted this revision.
bogner added a comment.
This revision is now accepted and ready to land.

Seems straightforward enough


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62693/new/

https://reviews.llvm.org/D62693



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50594: [analyzer] [NFC] Introduce separate targets for testing the analyzer: check-clang-analyzer and check-clang-analyzer-z3

[Justin Bogner via Phabricator via cfe-commits]

bogner added a comment.

The cmake and lit bits all look correct


https://reviews.llvm.org/D50594



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D49771: CodeGen: use non-zero memset when possible for automatic variables

[Justin Bogner via Phabricator via cfe-commits]

bogner accepted this revision.
bogner added a comment.
This revision is now accepted and ready to land.

Seems straightforward and correct to me.




Comment at: lib/CodeGen/CGDecl.cpp:956-957
+class BytePattern {
+  uint8_t Val;
+  enum class ValueType { Specific, Any, None } Type;
+  BytePattern(ValueType Type) : Type(Type) {}

Probably makes sense to swap the order of these or give the enum class a 
smaller underlying type than int.



Comment at: lib/CodeGen/CGDecl.cpp:996-998
+if (!Value.isSplat(8))
+  return BytePattern::None();
+return BytePattern(Value.getLoBits(8).getLimitedValue());

Very much a nitpick, but this would be slightly easier to follow written in the 
order without a negation.


Repository:
  rC Clang

https://reviews.llvm.org/D49771



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D42887: [Driver] Add option to manually control discarding value names in LLVM IR.

[Justin Bogner via Phabricator via cfe-commits]

bogner added inline comments.



Comment at: lib/Driver/ToolChains/Clang.cpp:3269-3274
+  const bool IsAssertBuild =
 #ifdef NDEBUG
-  CmdArgs.push_back("-disable-llvm-verifier");
-  // Discard LLVM value names in -asserts builds.
-  CmdArgs.push_back("-discard-value-names");
+  false;
+#else
+  true;
 #endif

It might be a few more characters, but I feel like this is more readable if you 
put entire statements in the branches of the #if, ie:

#ifdef NDEBUG
  const bool IsAssertBuild = false;
#else
  const bool IsAssertBuild = true;
#endif


https://reviews.llvm.org/D42887



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits