[clang] cb6fe61 - [Driver][DXC] Handle -Fo and -Fc flags

Justin Bogner via cfe-commits cfe-commits at lists.llvm.org
Tue Aug 15 16:39:23 PDT 2023


Author: Justin Bogner
Date: 2023-08-15T16:37:17-07:00
New Revision: cb6fe61be3dee67bb8b080c73dd2c48f2d0ce2c7

URL: https://github.com/llvm/llvm-project/commit/cb6fe61be3dee67bb8b080c73dd2c48f2d0ce2c7
DIFF: https://github.com/llvm/llvm-project/commit/cb6fe61be3dee67bb8b080c73dd2c48f2d0ce2c7.diff

LOG: [Driver][DXC] Handle -Fo and -Fc flags

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.

Differential Revision: https://reviews.llvm.org/D157582

Added: 
    clang/test/Driver/dxc_output.hlsl

Modified: 
    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
    llvm/lib/MC/MCParser/AsmParser.cpp

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index e0c5c6c0c2ff49..4ba3b237b80e58 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -8146,8 +8146,10 @@ class DXCJoinedOrSeparate<string name> : Option<["/", "-"], name,
 
 def dxc_no_stdinc : DXCFlag<"hlsl-no-stdinc">,
   HelpText<"HLSL only. Disables all standard includes containing non-native compiler types and functions.">;
-def Fo : DXCJoinedOrSeparate<"Fo">, Alias<o>,
+def dxc_Fo : DXCJoinedOrSeparate<"Fo">,
   HelpText<"Output object file">;
+def dxc_Fc : DXCJoinedOrSeparate<"Fc">,
+  HelpText<"Output assembly listing file">;
 def dxil_validator_version : Option<["/", "-"], "validator-version", KIND_SEPARATE>,
   Group<dxc_Group>, Flags<[NoXarchOption, HelpHidden]>,
   Visibility<[DXCOption, ClangOption, CC1Option]>,

diff  --git a/clang/include/clang/Driver/Types.def b/clang/include/clang/Driver/Types.def
index aaea3ec0f9c888..519d51f8ad1e5f 100644
--- a/clang/include/clang/Driver/Types.def
+++ b/clang/include/clang/Driver/Types.def
@@ -54,7 +54,7 @@ TYPE("objective-c++-cpp-output", PP_ObjCXX,    INVALID,         "mii",    phases
 TYPE("objc++-cpp-output",        PP_ObjCXX_Alias, INVALID,      "mii",    phases::Compile, phases::Backend, phases::Assemble, phases::Link)
 TYPE("objective-c++",            ObjCXX,       PP_ObjCXX,       "mm",     phases::Preprocess, phases::Compile, phases::Backend, phases::Assemble, phases::Link)
 TYPE("renderscript",             RenderScript, PP_C,            "rs",     phases::Preprocess, phases::Compile, phases::Backend, phases::Assemble, phases::Link)
-TYPE("hlsl",                     HLSL,         PP_CXX,          "hlsl",   phases::Preprocess, phases::Compile, phases::Backend)
+TYPE("hlsl",                     HLSL,         PP_CXX,          "hlsl",   phases::Preprocess, phases::Compile, phases::Backend, phases::Assemble)
 
 // C family input files to precompile.
 TYPE("c-header-cpp-output",      PP_CHeader,   INVALID,         "i",      phases::Precompile)

diff  --git a/clang/include/clang/Driver/Types.h b/clang/include/clang/Driver/Types.h
index 4a21af3534deba..121b58a6b477d9 100644
--- a/clang/include/clang/Driver/Types.h
+++ b/clang/include/clang/Driver/Types.h
@@ -43,7 +43,7 @@ namespace types {
 
   /// getTypeTempSuffix - Return the suffix to use when creating a
   /// temp file of this type, or null if unspecified.
-  const char *getTypeTempSuffix(ID Id, bool CLMode = false);
+  const char *getTypeTempSuffix(ID Id, bool CLStyle = false);
 
   /// onlyPrecompileType - Should this type only be precompiled.
   bool onlyPrecompileType(ID Id);

diff  --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp
index 3947ae949014d7..ede9967d8cef26 100644
--- a/clang/lib/Driver/Driver.cpp
+++ b/clang/lib/Driver/Driver.cpp
@@ -484,6 +484,10 @@ DerivedArgList *Driver::TranslateInputArgs(const InputArgList &Args) const {
     DAL->append(A);
   }
 
+  // DXC mode quits before assembly if an output object file isn't specified.
+  if (IsDXCMode() && !Args.hasArg(options::OPT_dxc_Fo))
+    DAL->AddFlagArg(nullptr, Opts.getOption(options::OPT_S));
+
   // Enforce -static if -miamcu is present.
   if (Args.hasFlag(options::OPT_miamcu, options::OPT_mno_iamcu, false))
     DAL->AddFlagArg(nullptr, Opts.getOption(options::OPT_static));
@@ -5023,7 +5027,8 @@ class ToolSelector final {
     return TC.useIntegratedAs() && !SaveTemps &&
            !C.getArgs().hasArg(options::OPT_via_file_asm) &&
            !C.getArgs().hasArg(options::OPT__SLASH_FA) &&
-           !C.getArgs().hasArg(options::OPT__SLASH_Fa);
+           !C.getArgs().hasArg(options::OPT__SLASH_Fa) &&
+           !C.getArgs().hasArg(options::OPT_dxc_Fc);
   }
 
   /// Return true if a preprocessor action can be collapsed.
@@ -5776,8 +5781,21 @@ const char *Driver::GetNamedOutputPath(Compilation &C, const JobAction &JA,
     return "-";
   }
 
-  if (IsDXCMode() && !C.getArgs().hasArg(options::OPT_o))
-    return "-";
+  if (JA.getType() == types::TY_PP_Asm &&
+      C.getArgs().hasArg(options::OPT_dxc_Fc)) {
+    StringRef FcValue = C.getArgs().getLastArgValue(options::OPT_dxc_Fc);
+    // TODO: Should we use `MakeCLOutputFilename` here? If so, we can probably
+    // handle this as part of the SLASH_Fa handling below.
+    return C.addResultFile(C.getArgs().MakeArgString(FcValue.str()), &JA);
+  }
+
+  if (JA.getType() == types::TY_Object &&
+      C.getArgs().hasArg(options::OPT_dxc_Fo)) {
+    StringRef FoValue = C.getArgs().getLastArgValue(options::OPT_dxc_Fo);
+    // TODO: Should we use `MakeCLOutputFilename` here? If so, we can probably
+    // handle this as part of the SLASH_Fo handling below.
+    return C.addResultFile(C.getArgs().MakeArgString(FoValue.str()), &JA);
+  }
 
   // Is this the assembly listing for /FA?
   if (JA.getType() == types::TY_PP_Asm &&
@@ -5791,6 +5809,11 @@ const char *Driver::GetNamedOutputPath(Compilation &C, const JobAction &JA,
         &JA);
   }
 
+  // DXC defaults to standard out when generating assembly. We check this after
+  // any DXC flags that might specify a file.
+  if (AtTopLevel && JA.getType() == types::TY_PP_Asm && IsDXCMode())
+    return "-";
+
   bool SpecifiedModuleOutput =
       C.getArgs().hasArg(options::OPT_fmodule_output) ||
       C.getArgs().hasArg(options::OPT_fmodule_output_EQ);
@@ -5809,7 +5832,8 @@ const char *Driver::GetNamedOutputPath(Compilation &C, const JobAction &JA,
       CCGenDiagnostics) {
     StringRef Name = llvm::sys::path::filename(BaseInput);
     std::pair<StringRef, StringRef> Split = Name.split('.');
-    const char *Suffix = types::getTypeTempSuffix(JA.getType(), IsCLMode());
+    const char *Suffix =
+        types::getTypeTempSuffix(JA.getType(), IsCLMode() || IsDXCMode());
     // The non-offloading toolchain on Darwin requires deterministic input
     // file name for binaries to be deterministic, therefore it needs unique
     // directory.
@@ -5900,7 +5924,8 @@ const char *Driver::GetNamedOutputPath(Compilation &C, const JobAction &JA,
     NamedOutput =
         MakeCLOutputFilename(C.getArgs(), Val, BaseName, types::TY_Object);
   } else {
-    const char *Suffix = types::getTypeTempSuffix(JA.getType(), IsCLMode());
+    const char *Suffix =
+        types::getTypeTempSuffix(JA.getType(), IsCLMode() || IsDXCMode());
     assert(Suffix && "All types used for output should have a suffix.");
 
     std::string::size_type End = std::string::npos;
@@ -5961,7 +5986,8 @@ const char *Driver::GetNamedOutputPath(Compilation &C, const JobAction &JA,
       StringRef Name = llvm::sys::path::filename(BaseInput);
       std::pair<StringRef, StringRef> Split = Name.split('.');
       std::string TmpName = GetTemporaryPath(
-          Split.first, types::getTypeTempSuffix(JA.getType(), IsCLMode()));
+          Split.first,
+          types::getTypeTempSuffix(JA.getType(), IsCLMode() || IsDXCMode()));
       return C.addTempFile(C.getArgs().MakeArgString(TmpName));
     }
   }

diff  --git a/clang/lib/Driver/ToolChains/HLSL.cpp b/clang/lib/Driver/ToolChains/HLSL.cpp
index 33f3ed1c638bfc..c6ad862b229420 100644
--- a/clang/lib/Driver/ToolChains/HLSL.cpp
+++ b/clang/lib/Driver/ToolChains/HLSL.cpp
@@ -229,14 +229,6 @@ HLSLToolChain::TranslateArgs(const DerivedArgList &Args, StringRef BoundArch,
     DAL->append(A);
   }
 
-  if (DAL->hasArg(options::OPT_o)) {
-    // When run the whole pipeline.
-    if (!DAL->hasArg(options::OPT_emit_llvm))
-      // Emit obj if write to file.
-      DAL->AddFlagArg(nullptr, Opts.getOption(options::OPT_emit_obj));
-  } else
-    DAL->AddSeparateArg(nullptr, Opts.getOption(options::OPT_o), "-");
-
   // Add default validator version if not set.
   // TODO: remove this once read validator version from validator.
   if (!DAL->hasArg(options::OPT_dxil_validator_version)) {

diff  --git a/clang/lib/Driver/Types.cpp b/clang/lib/Driver/Types.cpp
index 7d6308d757bc70..08df34ade7b653 100644
--- a/clang/lib/Driver/Types.cpp
+++ b/clang/lib/Driver/Types.cpp
@@ -80,8 +80,8 @@ types::ID types::getPrecompiledType(ID Id) {
   return TY_INVALID;
 }
 
-const char *types::getTypeTempSuffix(ID Id, bool CLMode) {
-  if (CLMode) {
+const char *types::getTypeTempSuffix(ID Id, bool CLStyle) {
+  if (CLStyle) {
     switch (Id) {
     case TY_Object:
     case TY_LTO_BC:

diff  --git a/clang/test/Driver/dxc_dxv_path.hlsl b/clang/test/Driver/dxc_dxv_path.hlsl
index c59c5ebbec7534..3d8e90d0d91975 100644
--- a/clang/test/Driver/dxc_dxv_path.hlsl
+++ b/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

diff  --git a/clang/test/Driver/dxc_output.hlsl b/clang/test/Driver/dxc_output.hlsl
new file mode 100644
index 00000000000000..6e94692e39be15
--- /dev/null
+++ b/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"

diff  --git a/llvm/lib/MC/MCParser/AsmParser.cpp b/llvm/lib/MC/MCParser/AsmParser.cpp
index 04590ed57a9f20..f70620c15a7ebb 100644
--- a/llvm/lib/MC/MCParser/AsmParser.cpp
+++ b/llvm/lib/MC/MCParser/AsmParser.cpp
@@ -807,7 +807,7 @@ AsmParser::AsmParser(SourceMgr &SM, MCContext &Ctx, MCStreamer &Out,
     PlatformParser.reset(createXCOFFAsmParser());
     break;
   case MCContext::IsDXContainer:
-    llvm_unreachable("DXContainer is not supported yet");
+    report_fatal_error("DXContainer is not supported yet");
     break;
   }
 


        


More information about the cfe-commits mailing list