This is an archive of the discontinued LLVM Phabricator instance.

[InstCombine] add multi-use demanded bits fold for add with low-bit mask
ClosedPublic

Authored by spatel on Nov 13 2020, 4:55 AM.

Details

Summary

I noticed an add example like the one from D91343, so here's a similar patch.
The logic is based on existing code for the single-use demanded bits fold. But I only matched a constant instead of using compute known bits on the operands because that was the motivating pattern.

I think this will allow removing a special-case (but incomplete) dedicated fold within visitAnd(), but I need to untangle the existing code to be sure.

https://rise4fun.com/Alive/V6fP

Name: add with low mask 
Pre: (C1 & (-1 u>> countLeadingZeros(C2))) == 0
%a = add i8 %x, C1
%r = and i8 %a, C2
=>
%r = and i8 %x, C2

Diff Detail

Event Timeline

spatel created this revision.Nov 13 2020, 4:55 AM
Herald added a project: Restricted Project. · View Herald TranscriptNov 13 2020, 4:55 AM
spatel requested review of this revision.Nov 13 2020, 4:55 AM
RKSimon added inline comments.Nov 13 2020, 9:07 AM
llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp
838

We could technically support non-uniform vectors here by matching m_Constant and using a similar ConstantExpr fold.

llvm/test/Transforms/InstCombine/and.ll
1052

Vector tests?

spatel marked an inline comment as done.Nov 13 2020, 10:11 AM
spatel added inline comments.
llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp
838

Ok - I didn't see any signs that we had tried non-uniform in this file yet. Do you think we should do that in this patch or as a TODO/follow-up?

llvm/test/Transforms/InstCombine/and.ll
1052

Basic splat is the last (positive) test.

RKSimon accepted this revision.Nov 14 2020, 9:18 AM

LGTM with a few minor comments

llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp
838

I'm happy for a TODO for now - but ideally we'd start getting as many combines supporting non-uniform constants (inc undef elements) as soon as possible.

Unfortunately I don't think llvm::MaskedValueIsZero will work for undef elements.

llvm/test/Transforms/InstCombine/and.ll
1052

cheers - if you could add the non-uniform test coverage now, even if support isn't added yet that'd be great.

This revision is now accepted and ready to land.Nov 14 2020, 9:18 AM

There is some uninitialized memory read http://lab.llvm.org:8011/#/builders/74/builds/888/steps/9/logs/stdio

FAIL: Clang :: SemaCXX/builtin-is-constant-evaluated.cpp (16913 of 71556)
******************** TEST 'Clang :: SemaCXX/builtin-is-constant-evaluated.cpp' FAILED ********************
Script:
--
: 'RUN: at line 1';   /b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm_build_msan/bin/clang -cc1 -internal-isystem /b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm_build_msan/lib/clang/12.0.0/include -nostdsysteminc -std=c++2a -verify /b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm-project/clang/test/SemaCXX/builtin-is-constant-evaluated.cpp -fcxx-exceptions -Wno-constant-evaluated -triple=x86_64-linux-gnu
--
Exit Code: 77
Command Output (stderr):
--
==14417==WARNING: MemorySanitizer: use-of-uninitialized-value
    #0 0x127ea7d4 in (anonymous namespace)::FloatExprEvaluator::VisitCastExpr(clang::CastExpr const*) /b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm-project/clang/lib/AST/ExprConstant.cpp:13686:3
    #1 0x127d7282 in clang::StmtVisitorBase<llvm::make_const_ptr, (anonymous namespace)::FloatExprEvaluator, bool>::Visit(clang::Stmt const*) /b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm-project/clang/include/clang/AST/StmtVisitor.h
    #2 0x126f42c1 in EvaluateFloat /b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm-project/clang/lib/AST/ExprConstant.cpp:13509:43
    #3 0x126f42c1 in Evaluate(clang::APValue&, (anonymous namespace)::EvalInfo&, clang::Expr const*) /b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm-project/clang/lib/AST/ExprConstant.cpp:14440:10
    #4 0x126e239e in EvaluateInPlace(clang::APValue&, (anonymous namespace)::EvalInfo&, (anonymous namespace)::LValue const&, clang::Expr const*, bool) /b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm-project/clang/lib/AST/ExprConstant.cpp:14525:10
    #5 0x126e4d4a in clang::Expr::EvaluateAsInitializer(clang::APValue&, clang::ASTContext const&, clang::VarDecl const*, llvm::SmallVectorImpl<std::__1::pair<clang::SourceLocation, clang::PartialDiagnostic> >&, bool) const /b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm-project/clang/lib/AST/ExprConstant.cpp:14820:10
    #6 0x124e0c6d in clang::VarDecl::evaluateValueImpl(llvm::SmallVectorImpl<std::__1::pair<clang::SourceLocation, clang::PartialDiagnostic> >&, bool) const /b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm-project/clang/lib/AST/Decl.cpp:2414:23
    #7 0x124e12b2 in clang::VarDecl::checkForConstantInitialization(llvm::SmallVectorImpl<std::__1::pair<clang::SourceLocation, clang::PartialDiagnostic> >&) const /b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm-project/clang/lib/AST/Decl.cpp:2485:7
    #8 0x10603eb6 in clang::Sema::CheckCompleteVariableDeclaration(clang::VarDecl*) /b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm-project/clang/lib/Sema/SemaDecl.cpp:13021:27
    #9 0x105fe616 in clang::Sema::AddInitializerToDecl(clang::Decl*, clang::Expr*, bool) /b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm-project/clang/lib/Sema/SemaDecl.cpp:12406:3
    #10 0xfe892f6 in clang::Parser::ParseDeclarationAfterDeclaratorAndAttributes(clang::Declarator&, clang::Parser::ParsedTemplateInfo const&, clang::Parser::ForRangeInit*) /b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm-project/clang/lib/Parse/ParseDecl.cpp:2311:17
    #11 0xfe819bd in clang::Parser::ParseDeclGroup(clang::ParsingDeclSpec&, clang::DeclaratorContext, clang::SourceLocation*, clang::Parser::ForRangeInit*) /b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm-project/clang/lib/Parse/ParseDecl.cpp:2037:21
    #12 0xfdeed21 in clang::Parser::ParseDeclOrFunctionDefInternal(clang::Parser::ParsedAttributesWithRange&, clang::ParsingDeclSpec&, clang::AccessSpecifier) /b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm-project/clang/lib/Parse/Parser.cpp:1134:10
    #13 0xfded513 in clang::Parser::ParseDeclarationOrFunctionDefinition(clang::Parser::ParsedAttributesWithRange&, clang::ParsingDeclSpec*, clang::AccessSpecifier) /b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm-project/clang/lib/Parse/Parser.cpp:1150:12
    #14 0xfdea89b in clang::Parser::ParseExternalDeclaration(clang::Parser::ParsedAttributesWithRange&, clang::ParsingDeclSpec*) /b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm-project/clang/lib/Parse/Parser.cpp:970:12
    #15 0xfe0b864 in clang::Parser::ParseInnerNamespace(llvm::SmallVector<clang::Parser::InnerNamespaceInfo, 4u> const&, unsigned int, clang::SourceLocation&, clang::ParsedAttributes&, clang::BalancedDelimiterTracker&) /b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm-project/clang/lib/Parse/ParseDeclCXX.cpp:246:7
    #16 0xfe08823 in clang::Parser::ParseNamespace(clang::DeclaratorContext, clang::SourceLocation&, clang::SourceLocation) /b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm-project/clang/lib/Parse/ParseDeclCXX.cpp:224:3
    #17 0xfe70478 in clang::Parser::ParseDeclaration(clang::DeclaratorContext, clang::SourceLocation&, clang::Parser::ParsedAttributesWithRange&, clang::SourceLocation*) /b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm-project/clang/lib/Parse/ParseDecl.cpp
    #18 0xfde94bb in clang::Parser::ParseExternalDeclaration(clang::Parser::ParsedAttributesWithRange&, clang::ParsingDeclSpec*) /b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm-project/clang/lib/Parse/Parser.cpp
    #19 0xfde5aec in clang::Parser::ParseTopLevelDecl(clang::OpaquePtr<clang::DeclGroupRef>&, bool) /b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm-project/clang/lib/Parse/Parser.cpp:716:12
    #20 0xfdd3c23 in clang::ParseAST(clang::Sema&, bool, bool) /b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm-project/clang/lib/Parse/ParseAST.cpp:158:20
    #21 0xbed5695 in clang::FrontendAction::Execute() /b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm-project/clang/lib/Frontend/FrontendAction.cpp:949:8
    #22 0xbcf8803 in clang::CompilerInstance::ExecuteAction(clang::FrontendAction&) /b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm-project/clang/lib/Frontend/CompilerInstance.cpp:991:33
    #23 0xc15ec6a in clang::ExecuteCompilerInvocation(clang::CompilerInstance*) /b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm-project/clang/lib/FrontendTool/ExecuteCompilerInvocation.cpp:278:25
    #24 0x325cd45 in cc1_main(llvm::ArrayRef<char const*>, char const*, void*) /b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm-project/clang/tools/driver/cc1_main.cpp:240:15
    #25 0x3255076 in ExecuteCC1Tool(llvm::SmallVectorImpl<char const*>&) /b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm-project/clang/tools/driver/driver.cpp:330:12
    #26 0x3253d3a in main /b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm-project/clang/tools/driver/driver.cpp:407:12
    #27 0x7f2d7579f09a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2409a)
    #28 0x31c8ef9 in _start (/b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm_build_msan/bin/clang-12+0x31c8ef9)
SUMMARY: MemorySanitizer: use-of-uninitialized-value /b/sanitizer-x86_64-linux-bootstrap-msan/build/llvm-project/clang/lib/AST/ExprConstant.cpp:13686:3 in (anonymous namespace)::FloatExprEvaluator::VisitCastExpr(clang::CastExpr const*)
Exiting

Is there anything I can re-use from that bot to reduce the repro burden? (I don't have a system set up to repro a 2-stage msan bug currently.)

I reverted to restore the bot. Getting nowhere fast trying to recreate that setup, so I'm not sure if it's worth the effort.
We already have a dedicated fold for the pattern shown in the tests, and it is artificially limited to single-use and scalar, so it's easier to just update that code.