[Driver][SYCL][FPGA] Allow for multiple AOCX device binaries for FPGA (#2909)

Initial implementation only allowed for a single AOCR/AOCX binary to
be used in a single FPGA compilation.  This restriction is being lifted
and we are allowing multiple FPGA device images to be added to the
final executable.

Allow for list file creation for aocx/aocr files with update extensions
when unbundling the archives.  To handle this, we are also enabling the
rename functionality for file-table-tform

Improve 'aoc' check in FPGA test for systems that have aoc installed
remove fpga_device_link_diag.cpp test, as we no longer do the spir check

Add TODO for improvement of output type and naming for aocx/aocr unbundles

Author: mdtoguchi

clang/include/clang/Driver/Action.h

@@ -717,6 +717,7 @@ class SPIRVTranslatorJobAction : public JobAction {
 // Provides a check of the given input file for the existence of SPIR kernel
 // code.  This is currently only used for FPGA specific tool chains and can
 // be expanded to perform other SPIR checks if needed.
+// TODO: No longer being used for FPGA (or elsewhere), cleanup needed.
 class SPIRCheckJobAction : public JobAction {
   void anchor() override;
 
@@ -779,7 +780,7 @@ class FileTableTformJobAction : public JobAction {
 
 public:
   struct Tform {
-    enum Kind { EXTRACT, EXTRACT_DROP_TITLE, REPLACE };
+    enum Kind { EXTRACT, EXTRACT_DROP_TITLE, REPLACE, RENAME };
 
     Tform() = default;
     Tform(Kind K, std::initializer_list<StringRef> Args) : TheKind(K) {
@@ -801,6 +802,10 @@ class FileTableTformJobAction : public JobAction {
   // <To> from another file table passed as input to this action.
   void addReplaceColumnTform(StringRef From, StringRef To);
 
+  // Renames a column with title <From> in this table with a column with title
+  // <To> passed as input to this action.
+  void addRenameColumnTform(StringRef From, StringRef To);
+
   static bool classof(const Action *A) {
     return A->getKind() == FileTableTformJobClass;
   }

clang/lib/Driver/Action.cpp

@@ -515,6 +515,11 @@ void FileTableTformJobAction::addReplaceColumnTform(StringRef From,
   Tforms.emplace_back(Tform(Tform::REPLACE, {From, To}));
 }
 
+void FileTableTformJobAction::addRenameColumnTform(StringRef From,
+                                                   StringRef To) {
+  Tforms.emplace_back(Tform(Tform::RENAME, {From, To}));
+}
+
 void StaticLibJobAction::anchor() {}
 
 StaticLibJobAction::StaticLibJobAction(ActionList &Inputs, types::ID Type)

clang/lib/Driver/Driver.cpp

@@ -3682,10 +3682,6 @@ class OffloadingActionBuilder final {
     /// construction. Does not track AOT binary inputs triples.
     SmallVector<llvm::Triple, 4> SYCLTripleList;
 
-    /// Running count of FPGA device binaries.
-    unsigned FPGAxCount = 0;
-    unsigned FPGArCount = 0;
-
     /// Type of output file for FPGA device compilation.
     types::ID FPGAOutType = types::TY_FPGA_AOCX;
 
@@ -3916,20 +3912,6 @@ class OffloadingActionBuilder final {
                                                   .isWindowsMSVCEnvironment()))
               FPGAObjectInputs.push_back(IA);
           }
-          // When creating FPGA device fat objects, all host objects are
-          // partially linked.  Gather that list here.
-          if (IA->getType() == types::TY_Object ||
-              IA->getType() == types::TY_FPGA_AOCX ||
-              IA->getType() == types::TY_FPGA_AOCR) {
-            // Keep track of the number of FPGA devices encountered
-            // Only one of these is allowed at a single time.
-            if (IA->getType() == types::TY_FPGA_AOCX)
-              FPGAxCount++;
-            if (IA->getType() == types::TY_FPGA_AOCR)
-              FPGArCount++;
-            if ((FPGAxCount && FPGArCount) || FPGAxCount > 1 || FPGArCount > 1)
-              C.getDriver().Diag(clang::diag::err_drv_bad_fpga_device_count);
-          }
         }
         for (unsigned I = 0; I < ToolChains.size(); ++I) {
           SYCLDeviceActions.push_back(UA);
@@ -4089,59 +4071,6 @@ class OffloadingActionBuilder final {
           // Current list is empty, nothing to process.
           continue;
 
-        // Perform a check for device kernels.  This is done for FPGA when an
-        // aocx or aocr based file is found.
-        if (FPGAxCount || FPGArCount) {
-          ActionList DeviceObjects;
-          for (const auto &I : LI) {
-            if (I->getType() == types::TY_Object) {
-              // Perform a check for SPIR kernel.
-              auto *DeviceCheckAction =
-                  C.MakeAction<SPIRCheckJobAction>(I, types::TY_Object);
-              DeviceObjects.push_back(DeviceCheckAction);
-              continue;
-            }
-            // We want to move the AOCX/AOCR binary to the front of the objects
-            // allowing it to be picked up instead of the other device objects
-            // at runtime.
-            // TODO: In the presense of existing FPGA Device binaries (AOCX)
-            // we do not need to perform/add the SPIR-V generated device
-            // binaries from sources or objects.
-            if (types::isFPGA(I->getType())) {
-              // Do not perform a device link and only pass the aocr
-              // file to the offline compilation before wrapping.  Just
-              // wrap an aocx file.
-              Action *DeviceWrappingAction;
-              if (I->getType() == types::TY_FPGA_AOCR) {
-                auto *DeviceBECompileAction =
-                    C.MakeAction<BackendCompileJobAction>(I, FPGAOutType);
-                DeviceWrappingAction = C.MakeAction<OffloadWrapperJobAction>(
-                    DeviceBECompileAction, types::TY_Object);
-              } else
-                DeviceWrappingAction =
-                    C.MakeAction<OffloadWrapperJobAction>(I, types::TY_Object);
-              DA.add(*DeviceWrappingAction, **TC, /*BoundArch=*/nullptr,
-                     Action::OFK_SYCL);
-              continue;
-            }
-            DeviceObjects.push_back(I);
-          }
-          if (!DeviceObjects.empty()) {
-            // When aocx or aocr is found, there is an expectation that none of
-            // the other objects processed have any kernel. So, there
-            // is no need in device code split and backend compile here. Just
-            // link and wrap the device binary.
-            auto *DeviceLinkAction =
-                C.MakeAction<LinkJobAction>(DeviceObjects, types::TY_LLVM_BC);
-            auto *SPIRVTranslateAction = C.MakeAction<SPIRVTranslatorJobAction>(
-                DeviceLinkAction, types::TY_SPIRV);
-            auto *DeviceWrappingAction = C.MakeAction<OffloadWrapperJobAction>(
-                SPIRVTranslateAction, types::TY_Object);
-            DA.add(*DeviceWrappingAction, **TC, /*BoundArch=*/nullptr,
-                   Action::OFK_SYCL);
-          }
-          continue;
-        }
         ActionList DeviceLibObjects;
         ActionList LinkObjects;
         auto TT = SYCLTripleList[I];
@@ -4153,9 +4082,33 @@ class OffloadingActionBuilder final {
           // FPGA aoco does not go through the link, everything else does.
           if (Input->getType() == types::TY_FPGA_AOCO)
             DeviceLibObjects.push_back(Input);
-          else
+          // FPGA aocr/aocx does not go through the link and is passed
+          // directly to the backend compilation step (aocr) or wrapper (aocx)
+          else if (types::isFPGA(Input->getType())) {
+            Action *FPGAAOTAction;
+            constexpr char COL_CODE[] = "Code";
+            constexpr char COL_ZERO[] = "0";
+            if (Input->getType() == types::TY_FPGA_AOCR)
+              // Generate AOCX/AOCR
+              FPGAAOTAction =
+                  C.MakeAction<BackendCompileJobAction>(Input, FPGAOutType);
+            else if (Input->getType() == types::TY_FPGA_AOCX)
+              FPGAAOTAction = Input;
+            else
+              llvm_unreachable("Unexpected FPGA input type.");
+            auto *RenameAction = C.MakeAction<FileTableTformJobAction>(
+                FPGAAOTAction, types::TY_Tempfilelist);
+            RenameAction->addRenameColumnTform(COL_ZERO, COL_CODE);
+            auto *DeviceWrappingAction = C.MakeAction<OffloadWrapperJobAction>(
+                RenameAction, types::TY_Object);
+            DA.add(*DeviceWrappingAction, **TC, /*BoundArch=*/nullptr,
+                   Action::OFK_SYCL);
+          } else
             LinkObjects.push_back(Input);
         }
+        if (LinkObjects.empty())
+          continue;
+
         // The linkage actions subgraph leading to the offload wrapper.
         // [cond] Means incoming/outgoing dependence is created only when cond
         //        is true. A function of:
@@ -5145,9 +5098,10 @@ void Driver::BuildActions(Compilation &C, DerivedArgList &Args,
     // archive unbundling for Windows.
     if (!isStaticArchiveFile(LA))
       continue;
-    // FPGA AOCX files are archives, but we do not want to unbundle them here
-    // as they have already been unbundled and processed for linking.
-    if (hasFPGABinary(C, LA.str(), types::TY_FPGA_AOCX))
+    // FPGA AOCX/AOCR files are archives, but we do not want to unbundle them
+    // here as they have already been unbundled and processed for linking.
+    if (hasFPGABinary(C, LA.str(), types::TY_FPGA_AOCX) ||
+        hasFPGABinary(C, LA.str(), types::TY_FPGA_AOCR))
       continue;
     // For offload-static-libs we add an unbundling action for each static
     // archive which produces list files with extracted objects. Device lists
@@ -6151,6 +6105,9 @@ InputInfo Driver::BuildJobsForActionNoCache(
             TI = types::TY_TempAOCOfilelist;
             Ext = "txt";
           }
+          if (JA->getType() == types::TY_FPGA_AOCR)
+            // AOCR files are always unbundled into a list file.
+            TI = types::TY_Tempfilelist;
         } else if (EffectiveTriple.getSubArch() !=
                    llvm::Triple::SPIRSubArch_fpga) {
           if (UI.DependentOffloadKind == Action::OFK_SYCL) {

clang/lib/Driver/ToolChains/Clang.cpp

@@ -7672,10 +7672,15 @@ void OffloadBundler::ConstructJobMultipleOutputs(
   bool IsFPGADepLibUnbundle = JA.getType() == types::TY_FPGA_Dependencies_List;
 
   if (InputType == types::TY_FPGA_AOCX || InputType == types::TY_FPGA_AOCR) {
-    // Override type with archive object
+    // Override type with AOCX/AOCR which will unbundle to a list containing
+    // binaries with the appropriate file extension (.aocx/.aocr).
+    // TODO - representation of the output file from the unbundle for these
+    // types (aocx/aocr) are always list files.  We should represent this
+    // better in the output extension and type for improved understanding
+    // of file contents and debuggability.
     if (getToolChain().getTriple().getSubArch() ==
         llvm::Triple::SPIRSubArch_fpga)
-      TypeArg = "ao";
+      TypeArg = InputType == types::TY_FPGA_AOCX ? "aocx" : "aocr";
     else
       TypeArg = "aoo";
   }
@@ -7884,7 +7889,8 @@ void OffloadWrapper::ConstructJob(Compilation &C, const JobAction &JA,
     const InputInfo &I = Inputs[0];
     assert(I.isFilename() && "Invalid input.");
 
-    if (I.getType() == types::TY_Tempfiletable)
+    if (I.getType() == types::TY_Tempfiletable ||
+        I.getType() == types::TY_Tempfilelist)
       // wrapper actual input files are passed via the batch job file table:
       WrapperArgs.push_back(C.getArgs().MakeArgString("-batch"));
     WrapperArgs.push_back(C.getArgs().MakeArgString(I.getFilename()));
@@ -8302,6 +8308,15 @@ void FileTableTform::ConstructJob(Compilation &C, const JobAction &JA,
       addArgs(CmdArgs, TCArgs, {Arg});
       break;
     }
+    case FileTableTformJobAction::Tform::RENAME: {
+      assert(Tf.TheArgs.size() == 2 && "from/to names expected");
+      SmallString<128> Arg("-rename=");
+      Arg += Tf.TheArgs[0];
+      Arg += ",";
+      Arg += Tf.TheArgs[1];
+      addArgs(CmdArgs, TCArgs, {Arg});
+      break;
+    }
     default:
       llvm_unreachable("unknown file table transformation kind");
     }

clang/lib/Driver/Types.cpp

@@ -254,65 +254,67 @@ bool types::isSrcFile(ID Id) {
 
 types::ID types::lookupTypeForExtension(llvm::StringRef Ext) {
   return llvm::StringSwitch<types::ID>(Ext)
-           .Case("c", TY_C)
-           .Case("C", TY_CXX)
-           .Case("F", TY_Fortran)
-           .Case("f", TY_PP_Fortran)
-           .Case("h", TY_CHeader)
-           .Case("H", TY_CXXHeader)
-           .Case("i", TY_PP_C)
-           .Case("m", TY_ObjC)
-           .Case("M", TY_ObjCXX)
-           .Case("o", TY_Object)
-           .Case("S", TY_Asm)
-           .Case("s", TY_PP_Asm)
-           .Case("bc", TY_LLVM_BC)
-           .Case("cc", TY_CXX)
-           .Case("CC", TY_CXX)
-           .Case("cl", TY_CL)
-           .Case("cp", TY_CXX)
-           .Case("cu", TY_CUDA)
-           .Case("hh", TY_CXXHeader)
-           .Case("ii", TY_PP_CXX)
-           .Case("ll", TY_LLVM_IR)
-           .Case("mi", TY_PP_ObjC)
-           .Case("mm", TY_ObjCXX)
-           .Case("rs", TY_RenderScript)
-           .Case("adb", TY_Ada)
-           .Case("ads", TY_Ada)
-           .Case("asm", TY_PP_Asm)
-           .Case("ast", TY_AST)
-           .Case("ccm", TY_CXXModule)
-           .Case("cpp", TY_CXX)
-           .Case("CPP", TY_CXX)
-           .Case("c++", TY_CXX)
-           .Case("C++", TY_CXX)
-           .Case("cui", TY_PP_CUDA)
-           .Case("cxx", TY_CXX)
-           .Case("CXX", TY_CXX)
-           .Case("F90", TY_Fortran)
-           .Case("f90", TY_PP_Fortran)
-           .Case("F95", TY_Fortran)
-           .Case("f95", TY_PP_Fortran)
-           .Case("for", TY_PP_Fortran)
-           .Case("FOR", TY_PP_Fortran)
-           .Case("fpp", TY_Fortran)
-           .Case("FPP", TY_Fortran)
-           .Case("gch", TY_PCH)
-           .Case("hip", TY_HIP)
-           .Case("hpp", TY_CXXHeader)
-           .Case("hxx", TY_CXXHeader)
-           .Case("iim", TY_PP_CXXModule)
-           .Case("lib", TY_Object)
-           .Case("mii", TY_PP_ObjCXX)
-           .Case("obj", TY_Object)
-           .Case("ifs", TY_IFS)
-           .Case("pch", TY_PCH)
-           .Case("pcm", TY_ModuleFile)
-           .Case("c++m", TY_CXXModule)
-           .Case("cppm", TY_CXXModule)
-           .Case("cxxm", TY_CXXModule)
-           .Default(TY_INVALID);
+      .Case("c", TY_C)
+      .Case("C", TY_CXX)
+      .Case("F", TY_Fortran)
+      .Case("f", TY_PP_Fortran)
+      .Case("h", TY_CHeader)
+      .Case("H", TY_CXXHeader)
+      .Case("i", TY_PP_C)
+      .Case("m", TY_ObjC)
+      .Case("M", TY_ObjCXX)
+      .Case("o", TY_Object)
+      .Case("S", TY_Asm)
+      .Case("s", TY_PP_Asm)
+      .Case("bc", TY_LLVM_BC)
+      .Case("cc", TY_CXX)
+      .Case("CC", TY_CXX)
+      .Case("cl", TY_CL)
+      .Case("cp", TY_CXX)
+      .Case("cu", TY_CUDA)
+      .Case("hh", TY_CXXHeader)
+      .Case("ii", TY_PP_CXX)
+      .Case("ll", TY_LLVM_IR)
+      .Case("mi", TY_PP_ObjC)
+      .Case("mm", TY_ObjCXX)
+      .Case("rs", TY_RenderScript)
+      .Case("adb", TY_Ada)
+      .Case("ads", TY_Ada)
+      .Case("asm", TY_PP_Asm)
+      .Case("ast", TY_AST)
+      .Case("ccm", TY_CXXModule)
+      .Case("cpp", TY_CXX)
+      .Case("CPP", TY_CXX)
+      .Case("c++", TY_CXX)
+      .Case("C++", TY_CXX)
+      .Case("cui", TY_PP_CUDA)
+      .Case("cxx", TY_CXX)
+      .Case("CXX", TY_CXX)
+      .Case("F90", TY_Fortran)
+      .Case("f90", TY_PP_Fortran)
+      .Case("F95", TY_Fortran)
+      .Case("f95", TY_PP_Fortran)
+      .Case("for", TY_PP_Fortran)
+      .Case("FOR", TY_PP_Fortran)
+      .Case("fpp", TY_Fortran)
+      .Case("FPP", TY_Fortran)
+      .Case("gch", TY_PCH)
+      .Case("hip", TY_HIP)
+      .Case("hpp", TY_CXXHeader)
+      .Case("hxx", TY_CXXHeader)
+      .Case("iim", TY_PP_CXXModule)
+      .Case("lib", TY_Object)
+      .Case("mii", TY_PP_ObjCXX)
+      .Case("obj", TY_Object)
+      .Case("ifs", TY_IFS)
+      .Case("pch", TY_PCH)
+      .Case("pcm", TY_ModuleFile)
+      .Case("c++m", TY_CXXModule)
+      .Case("cppm", TY_CXXModule)
+      .Case("cxxm", TY_CXXModule)
+      .Case("aocr", TY_FPGA_AOCR)
+      .Case("aocx", TY_FPGA_AOCX)
+      .Default(TY_INVALID);
 }
 
 types::ID types::lookupTypeForTypeSpecifier(const char *Name) {