Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
Fix the typo, then this looks good to me.
llvm/include/llvm/IR/Intrinsics.h | ||
---|---|---|
137–139 | NIT: TYPO: "There si no" -> "There is no" |
llvm/lib/IR/ConstantFold.cpp | ||
---|---|---|
922–923 | Just a thought for code like this - since we're now calling ElementCount::get that means we can now use the 'auto' keyword in such places. This might help reduce the character count and help to fit on one line? |
Thank you for the review @david-arm.
llvm/lib/IR/ConstantFold.cpp | ||
---|---|---|
922–923 | Good point! Done. |
@ftynse @mehdi_amini @JDevlieghere - apologies for breaking your workflows, and thank you for acting so quickly. I started doing the revert myself as soon as I saw the bombing from buildbot but by the time I was ready to push my revert your fixes were already pouring in.
I believe this can now be considered resolved. The patch from @mehdi_amini at https://github.com/llvm/llvm-project/commit/a407ec9b6db1e29e9aa361819f499ad11038d2dd is making sure that all subprojects are happy.
Thank you for your help.
Francesco
Why were static "get" methods preferred over simply marking the constructor explicit? For example, if the constructor is marked explicit, then this is what users see:
llvm::ElementCount EC = {vector->getNumElements(), /*scalable*/ false}; ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /home/dave/s/n/llvm/include/llvm/Support/TypeSize.h:37:12: note: explicit constructor declared here explicit ElementCount(unsigned Min, bool Scalable) : Min(Min), Scalable(Scalable) {} ^ 1 error generated.
Hi @davezarzycki ,
thank you for your comment. The idea of introducing get* methods was to make sure the users would make a conscious decision about whether they where setting up Fixed o Scalable instances of ElementCount. As for the general constructor you mention, we could have 1. allowed people to use initializer lists for ElementCount or alternatively 2. mark the constructor as explicit. With @ctetreau we decided to have an uniform approach, so we decided to allow building instances of ElementCount only via static methods. So, no real technical reason for not using "explicit", other than our preference.
Kind regards,
Francesco
use getFixed