pratyai added a comment. In D77244#1970820 <https://reviews.llvm.org/D77244#1970820>, @vitalybuka wrote:
> In D77244#1956930 <https://reviews.llvm.org/D77244#1956930>, @pratyai wrote: > > > It looks like I broke the tests after the `i8 `-> `i1` switch. > > > > I think it's because of an existing bug. From > > https://llvm.org/docs/LangRef.html > > > > > i1:8:8 - i1 is 8-bit (byte) aligned > > > > OTOH, in `SanitizerCoverage.cpp`, we have in > > `CreateFunctionLocalArrayInSection()`: > > > > Array->setAlignment(Align(Ty->isPointerTy() > > ? DL->getPointerSize() > > : Ty->getPrimitiveSizeInBits() / 8)); > > > > > > IIUC `getPrimitiveSizeInBits() / 8` would be `1 / 8 => 0` for `i1` type (it > > works for other `int` types which have multiple-of-8 bits. > > > > PLMK if my assessment is correct, and if so if I should fix it in a > > separate patch, or just keep that in here. > > > > I'll leave this patch "un-split" for now. > > > > Thanks! > > > Looks like a bug. > Separate patch would be nice. If it goes before this one we can expect NOP. > However, if you don't have committer access, I am going to submit this for > you, and I can split the patch myself. That'd be cool, thanks! ================ Comment at: llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp:922 + auto Store = IRB.CreateStore(ConstantInt::getTrue(Int1Ty), FlagPtr); + Store->setAtomic(AtomicOrdering::NotAtomic); + Store->setAlignment(llvm::MaybeAlign(FunctionBoolArray->getAlign())); ---------------- vitalybuka wrote: > Store->setAtomic(AtomicOrdering::NotAtomic); should be defaul Right; dropping. ================ Comment at: llvm/lib/Transforms/Instrumentation/SanitizerCoverage.cpp:923 + Store->setAtomic(AtomicOrdering::NotAtomic); + Store->setAlignment(llvm::MaybeAlign(FunctionBoolArray->getAlign())); + SetNoSanitizeMetadata(Store); ---------------- vitalybuka wrote: > isn't alignment is always 1 which is already default? > Right; the default is NoneType::None, which is already 1. Dropping. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77244/new/ https://reviews.llvm.org/D77244 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
