[PATCH] D64095: SVFS implementation according to RFC: Interface user provided vector functions with the vectorizer.

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert added a comment.

I think someone else should start looking at this.



================
Comment at: llvm/include/llvm/Analysis/SearchVectorFunctionSystem.h:137
+                .Case("Us", ParameterKind::OMP_LinearUValPos)
+                .Case("u",  ParameterKind::OMP_Uniform);
+        }
----------------
aranisumedh wrote:
> jdoerfert wrote:
> > The default case is missing. `llvm_unreachable` I assume.
> I don't think StringSwitch Default Case handles `llvm_unreachable`
> 
I guess you can do this: add a default case, if that value is chosen invoke llvm_unreachable.


================
Comment at: llvm/include/llvm/Analysis/SearchVectorFunctionSystem.h:121
+struct SVFS {
+  Module *M;
+
----------------
aranisumedh wrote:
> jdoerfert wrote:
> > I found a single use of this exposed member and that use can be replaced by something like:
> > `Call->getModule()`
> > If there is no reason to have this member, get rid of it and all the `init` stuff.
> I did make this change but however, as I'm building the CallInst programmatically in the test case, this leads to a segmentation fault. 
> 
> However, in the patch that is to follow(hooking to the loopVectorizer), Call->getModule() works perfectly. 
> 
> Do you have any suggestions as so as to get the getVectorizedFunction test working as specified in SearchVectorFunctionTest.cpp
> 
> Also, do I use AssertionSuccess() and AssertionFailure() for testing bad inputs?
You lost me here.


================
Comment at: llvm/unittests/Analysis/SearchVectorFunctionSystemTest.cpp:273
+  outs() << Call->getModule() << "\n";
+  outs() << M.get() << "\n";
+
----------------
I doubt we want to output anything here.


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

https://reviews.llvm.org/D64095