[PATCH] D64111: Add getMostFrequentByte and use for isBytewiseValue implementation

[Vitaly Buka via Phabricator via llvm-commits]

vitalybuka marked an inline comment as done.
vitalybuka added inline comments.


================
Comment at: llvm/lib/Analysis/ValueTracking.cpp:3206
+  if (C->isNullValue()) {
+    H.Values[0] += Size;
+    return;
----------------
bjope wrote:
> Not sure that we are guaranteed to get Size number of zeroes here. Not unless we also add a check for `DL.typeSizeEqualsStoreSize(V->getType)`. Consider if we for example have an i12, then only 12 bits out of 16 is known to be zero.
> 
> (this might be a bug on trunk as well, or maybe I've missed some condition somewhere regarding how this is used)
> 
> 
> 
yes, it's not precise, as not precise the ConstantFP handling below and isSplat(8) handling in ConstantInt
I guess it does not change behavior of IsBytewiseValueTest, and it's good enough for my future patch.

Also I think this is the best behavior for current users. If we memset zero there then we should be good. Same does not work below for  (CI->getBitWidth() % 8) and for non zero initiazer, we had to give up and count it against Other.

If we count non aligned isNullValue against "Other", it will change behavior of IsBytewiseValueTest. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64111