[PATCH] D52092: [ValueTracking] Generalize isBytewiseValue into isSplatValue

[Bjorn Pettersson via Phabricator via llvm-commits]

bjope added inline comments.


================
Comment at: include/llvm/Analysis/ValueTracking.h:229
+  /// truncated to one byte. Otherwise return null.
+  inline Value *isBytewiseValue(Value *V) {
+    return isSplatValue(V, 8);
----------------
jfb wrote:
> bjope wrote:
> > jfb wrote:
> > > Your target has 16-bit bytes, shouldn't this function (with your change) call `isSplatValue(V, 16)` in your target?
> > Correct! We are actually  doing `return isSplatValue(V, DataLayout::getBitsPerByte());` in our llvm fork. But I can't do that in llvm trunk, because there is currently no way to ask for getBitsPerByte().
> > 
> > Currently we also need to implement/maintain our own version of isSplatValue, because the existing isBytewiseValue function in llvm trunk is not general enough.
> Do you have plans to upstream your fork? It's otherwise pretty difficult to semi-support it, and it'll break very easily with changes that are useful to the LLVM community. It sounds like you're trying to incrementally get to a point where you can upstream?
I guess it will be hard to upstream full 16-bit bytes unless someone else is interested. There is not backend that supports it in tree, so it would be hard to create test cases. 
Our target/backend is for special hardware so that will never go public.

This time I was just trying to reduce some of the diffs we have compared to upstream. The idea was that our more general isSplatValue instead of (or rather as a complement to) isBytewiseValue could be useful to the LLVM community as well.

But this is not uttermost important for us. It is just one out of many functions that we need to override to support 16-bit bytes. We are fully aware of the complexity with maintaining this in our fork. But we do not have that many alternatives. Rebasing to trunk everyday, and running lots of compiler tests keeps your mobile network happy! (that could be a new company slogan...)


Repository:
  rL LLVM

https://reviews.llvm.org/D52092