[PATCH] D81500: [SVE] Remove calls to VectorType::getNumElements from IR

Christopher Tetreault via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Aug 25 13:27:27 PDT 2020


ctetreau added a comment.

@sdesmalen, I just wanted to touch bases on this. My understanding is that your concerns with this patch were largely driven by the fact that you and your team were crunching to get ACLE in master prior to the LLVM 11 branch point, and that by adding a bunch of assertion failures, my work was making yours more difficult. However, now that the branch point has come and gone, these concerns are hopefully lessened. If you have any concrete improvements to this, or any of the other patches I have up to remove calls to getNumElements(), feel free to let me know. Otherwise, I would really appreciate if we could get these patches reviewed and merged so I can stop playing whack-a-mole with this function :)

As mentioned, I acknowledge that on some level, substituting a miscompile with a crash is actually a behavior change. However, my goal has been to prevent different control flow paths from being taken after the point of the bug. At the point of a call to getNumElements(), either the compiler will crash if it gets a scalable vector, or it will behave the same as it did before. A function that used to return true should not start returning false without a test case. If I have failed in this goal, any reviewer should feel free to point it out and I will fix it.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81500/new/

https://reviews.llvm.org/D81500



More information about the llvm-commits mailing list