[PATCH] D145509: [HIP] Fix temporary files

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl marked an inline comment as done.
yaxunl added inline comments.


================
Comment at: clang/include/clang/Driver/Driver.h:634
+                             StringRef BoundArch = {},
+                             Action::OffloadKind OFK = Action::OFK_None) const;
 
----------------
tra wrote:
> I don't think that commingling offloading with something as generic as temp file creation is a good choice here.
> 
> I think we may want to coalesce both MultipleArchs and OFK into a more generic `Mode` argument which would have knobs for UNIQUE_DIRECTORY and MULTIPLE_ARCH.
> I suspect MULTIPLE_ARCH may be redundant -- we may make BoundArch an `optional<StringRef>` which would be a somewhat cleaner way to indicate whether file. Or just rely on BoundArch.empty().
> 
> Then figure out the right mode at the call site, where we'd have more context for what we do and why without having to propagate that knowledge to `CreateTempFile`.
> 
> So, roughly something like this:
> 
> ```
> const char *CreateTempFile(Compilation &C, StringRef Prefix, StringRef Suffix,
>                              StringRef BoundArch = {},
>                              bool UniqueDirectory = false) const;
> 
> ....
>     // MacOS must have stable file name, because reasons.
>     bool NeedsUniqueDir = (OFK == Action::OFK_None || OFK == Action::OFK_Host) && Triple.isOSDarwin());
>     return CreateTempFile(C, Split.first, Suffix, MultipleArchs ? BoundArch : "", NeedsUniqueDir);
> ```
will do


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

https://reviews.llvm.org/D145509