[PATCH] D98734: [dfsan] Add -dfsan-fast-8-labels flag

[stephan.yichao.zhao via Phabricator via llvm-commits]

stephan.yichao.zhao added inline comments.


================
Comment at: llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp:2468
   if (LeftSize >= ShadowVecSize) {
+    assert(ShadowVecSize * DFS.ShadowWidthBits <= 128);
     auto *ShadowVecTy =
----------------
gbalats wrote:
> stephan.yichao.zhao wrote:
> > The assert prevents using ShadowBytes > 2.
> > But not every number less than 128 works.
> > Maybe calculating  ShadowVecSize = 8bitmode? 64/ShadowWidthBits: 128/ShadowWidthBits; makes this easy to follow.
> I'm not sure why the suggestion makes this easier to follow. It only makes it easier if you know what the code was before, imo, but I think the original intention was that the vector type should fit in 128 bits regardless of the mode being used (which is what this assertion is trying to enforce).
> 
> Removed the assertion to support ShadowBytes > 2.
My first comment was confusing. It was not suggesting supporting ShadowBytes>2.
The first version with assertion(ShadowBytes * ShadowVecSize <= 128) is better, because with that the code can only use 64bit or 128bit.
Please add that assertion back. sorry for the confusing suggestion.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98734