jhuber6 marked an inline comment as done.
jhuber6 added inline comments.
================
Comment at: clang/lib/Driver/Driver.cpp:4391
DDep.add(*PackagerAction, *C.getSingleOffloadToolChain<Action::OFK_Host>(),
- nullptr, Action::OFK_None);
+ nullptr, C.getActiveOffloadKinds());
}
----------------
tra wrote:
> `getActiveOffloadKinds` returns a mask of offload kinds, yet we cast it to
> `OffloadKind` in `DeviceDependences::add` above. This mixing of OffloadKind
> and sets of them looks questionable to me.
>
> If we are relying on passing sets of `OffloadKind`now, then we should make it
> clear in the code that it is the intent, including a more detailed
> description that `DeviceOffloadKinds` is a list of `OffloadKind` sets, so
> whoever iterates over elements does not attempt to compare its elements with
> `OffloadKind` values. I think it would be helpful to change
> `DeviceOffloadKinds` element type to a new class with a helper method
> `hasOffloadKind(OffloadKind)` to avoid accidental comparison of a mask with
> enum value.
>
>
Yeah I think I shouldn't mix the logic here. I updated it.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D134189/new/
https://reviews.llvm.org/D134189
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
