[PATCH] D79421: [CodeGen] Fix FoldConstantVectorArithmetic for scalable vectors

[David Sherwood via Phabricator via llvm-commits]

david-arm marked an inline comment as done.
david-arm added a comment.

Re NIT1: all I mean is that these tests exercise these code paths. Previously we were fortunate enough that it just "worked", but now I've added to *ensure* that it is guaranteed to work. So in that sense the tests are testing that we don't do something silly in the places I've changed.



================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp:4946
 
+  // TODO: All the folds below are performed lane-by-lane and assume a fixed
+  // vector width, however we should be able to do constant folds involving
----------------
fpetrogalli wrote:
> NIT2: In the spirit of NIT1, I think you should add the TODO only on the part that requires implementing constant folding for splat into scalable vectors, as follows:
> 
> ```
> // Custom code for scalable vectors, as all the folds below assume a fixed width vector.
> // TODO: bail off for now, however we should be able to do constant folds involving splats to scalable vectors too.
> ``` 
> 
> This way, once the TODO is done and removed, we will not loose the information on why the scalable vectors require custom code.
Hi Francesco, I'm sorry but I don't quite follow here. All the code below this is not custom code for scalable vectors. Even if we later fixed the code below to work for both fixed and scalable, it still won't be specific to scalable.


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

https://reviews.llvm.org/D79421