agozillon added inline comments.
================
Comment at: clang/lib/Driver/ToolChains/Flang.cpp:128
+ if (IsHostOffloadingAction) {
+ for (size_t i = 1; i < Inputs.size(); ++i) {
+ if (Inputs[i].getType() != types::TY_Nothing)
----------------
jhuber6 wrote:
> agozillon wrote:
> > awarzynski wrote:
> > > agozillon wrote:
> > > > awarzynski wrote:
> > > > > What's the magic "1"? And given that the input count matters here -
> > > > > is there a test with multiple inputs?
> > > > It aims to mimic the behavior of Clang:
> > > > https://github.com/llvm/llvm-project/blob/main/clang/lib/Driver/ToolChains/Clang.cpp#L4561
> > > > where the main input is skipped (the input currently being compiled or
> > > > embedded into etc.), when adding to //-fembed-offload-object//.
> > > >
> > > > It does look different to Clang's as Clang has more cases and the logic
> > > > is spread across the constructJob invocation, but the first if case is
> > > > what the if statement inside of the loop and setting the loop index
> > > > variable to 1 do. The HostOffloadingInputs array is what is being
> > > > generated here, except we're skipping and directly applying it as
> > > > arguments.
> > > >
> > > > I tried to condense it a little in this case! Perhaps it loses
> > > > readability though, I had hoped the comment might have kept it clear
> > > Thanks for the link - that code in Clang doesn't really clarify what
> > > makes `Inputs[0]` special 🤔 .
> > >
> > > Let me rephrase my question - what's so special about the first input?
> > > (referred to in Clang as "main input") Is that something specific to
> > > OpenMP? For example, in this case:
> > > ```
> > > flang-new -fopenmp file.f90
> > > ```
> > > I assume that `inputs[0]` is "file.f90", so nothing will happen?
> > >
> > > > I tried to condense it a little in this case! Perhaps it loses
> > > > readability though, I had hoped the comment might have kept it clear
> > >
> > > Nah, I think that your implementation is fine. It's my ignorance with
> > > respect to OpenMP that's the problem here ;-)
> > It's not specific to OpenMP I believe, as far as I am aware Clang's
> > supported offload models (SYCL and CUDA as well as OpenMP) use it! In
> > Flang's case we only really care about OpenMP as I believe it's the only
> > offload programming model supported.
> >
> > In the case of the command:
> >
> > ```
> > flang-new -fopenmp file.f90
> > ```
> > The code should never be executed as no part of the command will enable an
> > offloading action (for device or host)! But yes inputs[0] would indeed
> > refer to file.f90.
> >
> > However, this code becomes relevant when you utilise an option that enables
> > the clangDriver to perform some form of offloading action. For example a
> > command like:
> >
> > ```
> > flang-new -fopenmp --offload-arch=gfx90a file.f90
> > ```
> > Will trigger two phase compilation, one for the host device (your resident
> > CPU in this command) and one for the device (gfx90a in this command), the
> > regular host pass will invoke like your provided command and the device
> > pass will then invoke with -fopenmp-is-device in addition alongside the
> > device triple. This generates two bitcode files from the one file, one
> > containing the host code from the file, the other the device code
> > (extracted from OpenMP target regions or declare target etc.).
> >
> > However, now we have two files, with both parts of our program, we need to
> > conjoin them together, the clangDriver generates an embeddable
> > fat-binary/binary using the clang-offload-packager and then invokes
> > flang-new again, and this is where the above code becomes relevant, as this
> > binary (or multiple binaries, if we target multiple devices in the same
> > program) is what is passed to -fembed-offload-object! And inputs[0] in this
> > case would refer to the output from the original host pass, which is what
> > we want to embed the device binary into, so we wish to skip this original
> > host output and only pass the subsequent inputs (which should be device
> > binaries when the clangDriver initiates a host offloading action) we want
> > to embed as -fembed-offload-object arguments.
> >
> > The offloading driver is quite complex and my knowledge of it is not
> > perfect as I am not our resident expert on it unfortunately (so if anyone
> > sees anything incorrect, please do chime in and correct me)!
> >
> > But hopefully this answers your question and gives you an idea of when and
> > how this -fembed-offload-object comes into play, essentially a way to get
> > the device binaries we wish to insert into the host binary, so it can load
> > the binaries at runtime and execute them. Currently upstream Flang doesn't
> > utilise this option of course, but we intend to use this as part of our
> > OpenMP offloading efforts for AMD devices (whilst leaving the door open for
> > other vendors devices as well). We are trying to re-use/mimic as much of
> > the existing machinery that the clangDriver implements as we can!
> >
> The compiler requires at least one input file to run, otherwise it exits
> early. Therefore we're guaranteed to have at least one input file in the
> list. Some functions need an input file, usually to write some temp name to,
> and `Inputs[0]` is the easiest way to get an input file.
Thank you very much @jhuber6! I should have added you as a subscriber/reviewer
as well in hindsight, sorry about that.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D145815/new/
https://reviews.llvm.org/D145815
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
