[PATCH] D121984: [RISCV][NFC] Moving RVV intrinsic type related util to llvm/Support

kito-cheng created this revision. Herald added subscribers: s, VincentWu, luke957, vkmr, frasercrmck, evandro, luismarques, apazos, sameer.abuasal, s.egerton, Jim, benna, psnobl, jocewei, PkmX, the_o, brucehoult, MartinMosbeck, rogfer01, edward-jones, zzheng, jrtc27, niosHD, sabuasal, simoncook,

[PATCH] D121984: [RISCV][NFC] Moving RVV intrinsic type related util to llvm/Support

khchen accepted this revision. khchen added a comment. This revision is now accepted and ready to land. LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121984/new/ https://reviews.llvm.org/D121984 __

[PATCH] D121984: [RISCV][NFC] Moving RVV intrinsic type related util to llvm/Support

kito-cheng added a comment. Oh, apology, I guess I just miss the discussion, let me revert that again. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121984/new/ https://reviews.llvm.org/D121984 ___ cfe-c

[PATCH] D121984: [RISCV][NFC] Moving RVV intrinsic type related util to llvm/Support

kito-cheng updated this revision to Diff 421477. kito-cheng added a comment. Changes: - Move those stuffs into clang/Support instead of llvm/Support - Keep table gen staffs in clang/utils/TableGen/RISCVVEmitter.cpp Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://revie

[PATCH] D121984: [RISCV][NFC] Moving RVV intrinsic type related util to llvm/Support

kito-cheng added a comment. @MaskRay I've move those stuffs into clang/Support which is introduce by this patch, and also keep those table gen stuffs in clang-tblgen, did you mind take a look? Thanks :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.or

[PATCH] D121984: [RISCV][NFC] Moving RVV intrinsic type related util to llvm/Support

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGad57e10dbca2: [RISCV][NFC] Moving RVV intrinsic type related util to llvm/Support (authored by kito-cheng). Changed prior to commit: https://revie

[PATCH] D121984: [RISCV][NFC] Moving RVV intrinsic type related util to llvm/Support

MaskRay added a comment. Seems that there is a circular dependency problem: 7c7e7770b7176f452d40f4e7ac545aaee1b19c4d (reverted). I'd suggest that we revert this patch. Having target specific stuff in LLVMSupport is always f

[PATCH] D121984: [RISCV][NFC] Moving RVV intrinsic type related util to llvm/Support

kito-cheng added a subscriber: akuegel. kito-cheng added a comment. Hi @MaskRay, @akuegel has removed the `llvm/TableGen/*`, so I guess the issue you mentioned in https://github.com/llvm/llvm-project/commit/c0eb9b4cdef6049ebabb4018d3c9dcb0dc699868 is resolved by https://github.com/llvm/llvm-pr

[PATCH] D121984: [RISCV][NFC] Moving RVV intrinsic type related util to llvm/Support

akuegel added a comment. Yes, I believe the issue with the circular dependency was already fixed by my latest commit. Sorry for first committing the wrong fix (just removing the headers without checking that they were needed to pull in raw_ostream header transitively). So if you apply my fix, y

[PATCH] D121984: [RISCV][NFC] Moving RVV intrinsic type related util to llvm/Support

MaskRay added a comment. Ah, sorry, I did not see that the end state of @akuegel's 3 commits fixed the circular dependency. However, I think my other concern still applies: the layering seems weird that the RISCV stuff is placed in LLVMSupport. Any chance it can be fixed? You may consider a mor

[PATCH] D121984: [RISCV][NFC] Moving RVV intrinsic type related util to llvm/Support

MaskRay added a comment. The original patch added 600 lines while the subject just said "Move". This may be fishy as well. Having TableGen related `void RVVIntrinsic::emitCodeGenSwitchBody(raw_ostream &OS) const {` in LLVMSupport is also weird. So I think this patch deserves more discussion an