[Openmp-commits] [PATCH] D63009: [OpenMP] Add target task alloc function with device ID
Alexey Bataev via Phabricator via Openmp-commits
openmp-commits at lists.llvm.org
Sun Jun 16 10:29:45 PDT 2019
ABataev added a comment.
In D63009#1545152 <https://reviews.llvm.org/D63009#1545152>, @gtbercea wrote:
> In D63009#1544984 <https://reviews.llvm.org/D63009#1544984>, @ABataev wrote:
> > In D63009#1544900 <https://reviews.llvm.org/D63009#1544900>, @gtbercea wrote:
> > > In D63009#1544758 <https://reviews.llvm.org/D63009#1544758>, @Hahnfeld wrote:
> > >
> > > > Am I correct that the second to last revision ("- Fix tests.") removed all checks for the actual `device_id` argument from the tests? From my point of view that's not fixing but weakening the tests! Can you explain why they needed "fixing"?
> > >
> > >
> > > When I was just passing the default value the LLVM-IR was: i64 -1 i.e. constant, easy to check.
> > >
> > > With the latest change the emitted code is: i64 %123 i.e. where %123 is a local derived from the expression of the device ID.
> > If the value is constant, check for the constant.
> All the tests here use expressions so the value is never constant. If you'd like me to add a test with constant I can.
Yes, there must be at least one test with the default value.
>> And at least several tests with the expressions should check for the correct value of the expression.
> I'll have to check how to do this. There's nothing that distinguishes an expression that represents the device ID from any other expression in the code.
CHANGES SINCE LAST ACTION
More information about the Openmp-commits