[PATCH] D88190: C API: functions to get mask of a ShuffleVector

Robert Widmann via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sat Sep 26 11:42:12 PDT 2020


CodaFi added inline comments.


================
Comment at: llvm/lib/IR/Core.cpp:3966
+}
+const int LLVMUndefMaskElem =
+    -1; // not actually accessible as ShuffleVectorInst::UndefMaskElem, so we
----------------
cdisselkoen wrote:
> CodaFi wrote:
> > Can we add a getter for this instead? We should avoid magic numbers in the C API - it doesn’t tend to stay in sync with the C++ otherwise
> Actually, as I was looking into this, I realized that we can just write `const int LLVMUndefMaskElem = UndefMaskElem;` here and it works.  `UndefMaskElem` is just a constant available in the `Instructions.h` header.
> 
> Since this has already landed on master, should I open a new diff with this change?  Or still modify this diff?
To both questions 

1) Please don’t mix C and C++ headers. It may happen to work for you locally now, but there’s no guarantee Instructions.h won’t break everybody in the future. Please make an explicit getter.

2) You should submit another revision for review here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88190



More information about the llvm-commits mailing list