mehdi_amini added a comment. The patch is pretty large, any chance you could split in two? First the change introducing the AttributeKey and changing the API, and then updating all the callees separately as a follow up?
================ Comment at: llvm/include/llvm/IR/Attributes.h:84 + } +}; ---------------- This whole code deserves documentation I think. ================ Comment at: llvm/lib/IR/Attributes.cpp:125 FoldingSetNodeID ID; - ID.AddString(Kind); + ID.AddString(Kind.value()); if (!Val.empty()) ID.AddString(Val); ---------------- Seems like this method does not use anything than the `StringRef` inside the Kind, why change it to take an `AttributeKey`? Won't you eagerly compute the hash even if not needed? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114082/new/ https://reviews.llvm.org/D114082 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
