jdoerfert added a comment. In D92439#2431270 <https://reviews.llvm.org/D92439#2431270>, @aaron.ballman wrote:
> In D92439#2431256 <https://reviews.llvm.org/D92439#2431256>, @jdoerfert wrote: > >> In D92439#2429815 <https://reviews.llvm.org/D92439#2429815>, @jyu2 wrote: >> >>> In D92439#2429511 <https://reviews.llvm.org/D92439#2429511>, @jdoerfert >>> wrote: >>> >>>> Still unsure if we should also error out for NVPTX but that is a different >>>> story. Looks OK from my side, assuming you address the earlier comment. >>> >>> With this change if NVPTX need diagnostic for use of 128-bit integer, >>> adding "bool hasInt128Type() const override { return false; }" in NVPTX.h >>> is all needed. >>> >>>> Maybe someone else should accept though. >>> >>> Do you have suggestion whom I may contact for acceptance? We have customer >>> needs for this... Thank you in advance. :-) >> >> You have the right people as reviewers, it sometimes need a few days. You >> can ping it after a week without progress. > > FWIW, the changes LGTM but I don't know enough about the domain to know the > answer for NVPTX. That said, this is still good incremental progress as-is. Leave NVPTX out for now. As I said, it looks like the backend does the right thing so it is "legal" to have i128. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92439/new/ https://reviews.llvm.org/D92439 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
