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. Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63009/new/ https://reviews.llvm.org/D63009 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
