[LICM] Support hoisting of non-argmemonly readonly calls

[nikic]

The code checking whether a readonly call is safe to hoist is
currently limited to only argmemonly calls. However, the actual
implementation does not depend on this in any way. It either
does an MSSA clobber walk on the memory access (which will take
all locations accessed by the call into account), or it will
look at all MemoryDefs in an entirely location-independent manner.

The current restriction dates back to the time when LICM still
supported AST, in which case this code did reason about the
individual pointer arguments.

[nikic]

argmemonly for a function without arguments is memory(none). Not LICM's job to figure that out...

[preames]

LGTM

[fhahn]

LGTM, thanks

[mikaelholmen]

Hi @nikic

I ran into a crash that I could bisect back to this patch.

opt -passes="function(interleaved-load-combine),cgscc(function-attrs),function(loop-mssa(licm))" bbi-109713.ll -S -o /dev/null -mtriple=hexagon

crashes in LICM with

opt: ../include/llvm/Support/Casting.h:578: decltype(auto) llvm::cast(From *) [To = llvm::MemoryUse, From = llvm::MemoryUseOrDef]: Assertion `isa<To>(Val) && "cast<Ty>() argument of incompatible type!"' failed.
PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace.
Stack dump:
0.	Program arguments: build-all/bin/opt -passes=function(interleaved-load-combine),cgscc(function-attrs),function(loop-mssa(licm)) bbi-109713.ll -S -o /dev/null -mtriple=hexagon
1.	Running pass "function(loop-mssa(licm<allowspeculation>))" on module "bbi-109713.ll"
2.	Running pass "loop-mssa(licm<allowspeculation>)" on function "f875"
 #0 0x0000559040539f46 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) (build-all/bin/opt+0x47eff46)
 #1 0x00005590405374d5 llvm::sys::RunSignalHandlers() (build-all/bin/opt+0x47ed4d5)
 #2 0x000055904053b0c9 SignalHandler(int, siginfo_t*, void*) Signals.cpp:0:0
 #3 0x00007f0fe5b55990 __restore_rt (/lib64/libpthread.so.0+0x12990)
 #4 0x00007f0fe34f552f raise (/lib64/libc.so.6+0x4e52f)
 #5 0x00007f0fe34c8e65 abort (/lib64/libc.so.6+0x21e65)
 #6 0x00007f0fe34c8d39 _nl_load_domain.cold.0 (/lib64/libc.so.6+0x21d39)
 #7 0x00007f0fe34ede86 (/lib64/libc.so.6+0x46e86)
 #8 0x0000559040f4c89e (build-all/bin/opt+0x520289e)
 #9 0x0000559040f45ace llvm::canSinkOrHoistInst(llvm::Instruction&, llvm::AAResults*, llvm::DominatorTree*, llvm::Loop*, llvm::MemorySSAUpdater&, bool, llvm::SinkAndHoistLICMFlags&, llvm::OptimizationRemarkEmitter*) (build-all/bin/opt+0x51fbace)
#10 0x0000559040f463d4 llvm::hoistRegion(llvm::DomTreeNodeBase<llvm::BasicBlock>*, llvm::AAResults*, llvm::LoopInfo*, llvm::DominatorTree*, llvm::AssumptionCache*, llvm::TargetLibraryInfo*, llvm::Loop*, llvm::MemorySSAUpdater&, llvm::ScalarEvolution*, llvm::ICFLoopSafetyInfo*, llvm::SinkAndHoistLICMFlags&, llvm::OptimizationRemarkEmitter*, bool, bool, bool) (build-all/bin/opt+0x51fc3d4)
#11 0x0000559040f40d55 (anonymous namespace)::LoopInvariantCodeMotion::runOnLoop(llvm::Loop*, llvm::AAResults*, llvm::LoopInfo*, llvm::DominatorTree*, llvm::AssumptionCache*, llvm::TargetLibraryInfo*, llvm::TargetTransformInfo*, llvm::ScalarEvolution*, llvm::MemorySSA*, llvm::OptimizationRemarkEmitter*, bool) LICM.cpp:0:0
#12 0x0000559040f4071e llvm::LICMPass::run(llvm::Loop&, llvm::AnalysisManager<llvm::Loop, llvm::LoopStandardAnalysisResults&>&, llvm::LoopStandardAnalysisResults&, llvm::LPMUpdater&) (build-all/bin/opt+0x51f671e)
#13 0x0000559041a01e4d llvm::detail::PassModel<llvm::Loop, llvm::LICMPass, llvm::AnalysisManager<llvm::Loop, llvm::LoopStandardAnalysisResults&>, llvm::LoopStandardAnalysisResults&, llvm::LPMUpdater&>::run(llvm::Loop&, llvm::AnalysisManager<llvm::Loop, llvm::LoopStandardAnalysisResults&>&, llvm::LoopStandardAnalysisResults&, llvm::LPMUpdater&) PassBuilderPipelines.cpp:0:0
#14 0x0000559041c99bbc std::optional<llvm::PreservedAnalyses> llvm::PassManager<llvm::Loop, llvm::AnalysisManager<llvm::Loop, llvm::LoopStandardAnalysisResults&>, llvm::LoopStandardAnalysisResults&, llvm::LPMUpdater&>::runSinglePass<llvm::Loop, std::unique_ptr<llvm::detail::PassConcept<llvm::Loop, llvm::AnalysisManager<llvm::Loop, llvm::LoopStandardAnalysisResults&>, llvm::LoopStandardAnalysisResults&, llvm::LPMUpdater&>, std::default_delete<llvm::detail::PassConcept<llvm::Loop, llvm::AnalysisManager<llvm::Loop, llvm::LoopStandardAnalysisResults&>, llvm::LoopStandardAnalysisResults&, llvm::LPMUpdater&>>>>(llvm::Loop&, std::unique_ptr<llvm::detail::PassConcept<llvm::Loop, llvm::AnalysisManager<llvm::Loop, llvm::LoopStandardAnalysisResults&>, llvm::LoopStandardAnalysisResults&, llvm::LPMUpdater&>, std::default_delete<llvm::detail::PassConcept<llvm::Loop, llvm::AnalysisManager<llvm::Loop, llvm::LoopStandardAnalysisResults&>, llvm::LoopStandardAnalysisResults&, llvm::LPMUpdater&>>>&, llvm::AnalysisManager<llvm::Loop, llvm::LoopStandardAnalysisResults&>&, llvm::LoopStandardAnalysisResults&, llvm::LPMUpdater&, llvm::PassInstrumentation&) (build-all/bin/opt+0x5f4fbbc)
#15 0x0000559041c997f3 llvm::PassManager<llvm::Loop, llvm::AnalysisManager<llvm::Loop, llvm::LoopStandardAnalysisResults&>, llvm::LoopStandardAnalysisResults&, llvm::LPMUpdater&>::runWithoutLoopNestPasses(llvm::Loop&, llvm::AnalysisManager<llvm::Loop, llvm::LoopStandardAnalysisResults&>&, llvm::LoopStandardAnalysisResults&, llvm::LPMUpdater&) (build-all/bin/opt+0x5f4f7f3)
#16 0x0000559041c98cf4 llvm::PassManager<llvm::Loop, llvm::AnalysisManager<llvm::Loop, llvm::LoopStandardAnalysisResults&>, llvm::LoopStandardAnalysisResults&, llvm::LPMUpdater&>::run(llvm::Loop&, llvm::AnalysisManager<llvm::Loop, llvm::LoopStandardAnalysisResults&>&, llvm::LoopStandardAnalysisResults&, llvm::LPMUpdater&) (build-all/bin/opt+0x5f4ecf4)
#17 0x00005590419ff8dd llvm::detail::PassModel<llvm::Loop, llvm::PassManager<llvm::Loop, llvm::AnalysisManager<llvm::Loop, llvm::LoopStandardAnalysisResults&>, llvm::LoopStandardAnalysisResults&, llvm::LPMUpdater&>, llvm::AnalysisManager<llvm::Loop, llvm::LoopStandardAnalysisResults&>, llvm::LoopStandardAnalysisResults&, llvm::LPMUpdater&>::run(llvm::Loop&, llvm::AnalysisManager<llvm::Loop, llvm::LoopStandardAnalysisResults&>&, llvm::LoopStandardAnalysisResults&, llvm::LPMUpdater&) PassBuilderPipelines.cpp:0:0
#18 0x0000559041c9ac01 llvm::FunctionToLoopPassAdaptor::run(llvm::Function&, llvm::AnalysisManager<llvm::Function>&) (build-all/bin/opt+0x5f50c01)
#19 0x0000559041a023cd llvm::detail::PassModel<llvm::Function, llvm::FunctionToLoopPassAdaptor, llvm::AnalysisManager<llvm::Function>>::run(llvm::Function&, llvm::AnalysisManager<llvm::Function>&) PassBuilderPipelines.cpp:0:0
#20 0x0000559040754215 llvm::PassManager<llvm::Function, llvm::AnalysisManager<llvm::Function>>::run(llvm::Function&, llvm::AnalysisManager<llvm::Function>&) (build-all/bin/opt+0x4a0a215)
#21 0x0000559041a0489d llvm::detail::PassModel<llvm::Function, llvm::PassManager<llvm::Function, llvm::AnalysisManager<llvm::Function>>, llvm::AnalysisManager<llvm::Function>>::run(llvm::Function&, llvm::AnalysisManager<llvm::Function>&) PassBuilderPipelines.cpp:0:0
#22 0x0000559040758dce llvm::ModuleToFunctionPassAdaptor::run(llvm::Module&, llvm::AnalysisManager<llvm::Module>&) (build-all/bin/opt+0x4a0edce)
#23 0x0000559041993f3d llvm::detail::PassModel<llvm::Module, llvm::ModuleToFunctionPassAdaptor, llvm::AnalysisManager<llvm::Module>>::run(llvm::Module&, llvm::AnalysisManager<llvm::Module>&) NewPMDriver.cpp:0:0
#24 0x0000559040752f05 llvm::PassManager<llvm::Module, llvm::AnalysisManager<llvm::Module>>::run(llvm::Module&, llvm::AnalysisManager<llvm::Module>&) (build-all/bin/opt+0x4a08f05)
#25 0x000055904198cd84 llvm::runPassPipeline(llvm::StringRef, llvm::Module&, llvm::TargetMachine*, llvm::TargetLibraryInfoImpl*, llvm::ToolOutputFile*, llvm::ToolOutputFile*, llvm::ToolOutputFile*, llvm::StringRef, llvm::ArrayRef<llvm::PassPlugin>, llvm::ArrayRef<std::function<void (llvm::PassBuilder&)>>, llvm::opt_tool::OutputKind, llvm::opt_tool::VerifierKind, bool, bool, bool, bool, bool, bool, bool, bool) (build-all/bin/opt+0x5c42d84)
#26 0x00005590404db738 optMain (build-all/bin/opt+0x4791738)
#27 0x00007f0fe34e17e5 __libc_start_main (/lib64/libc.so.6+0x3a7e5)
#28 0x00005590404d916e _start (build-all/bin/opt+0x478f16e)
Abort (core dumped)

It's the cast below that fails:

    if (Behavior.onlyReadsMemory()) {
      // If we can prove there are no writes to the memory read by the call, we
      // can hoist or sink.
      return !pointerInvalidatedByLoop(
          MSSA, cast<MemoryUse>(MSSA->getMemoryAccess(CI)), CurLoop, I, Flags,
          /*InvariantGroup=*/false);
    }

bbi-109713.ll.gz

[nikic]

@mikaelholmen Thanks for the report. This should be fixed by 5753ee2 and #154289 would avoid the situation in the first place.

[mikaelholmen]

@mikaelholmen Thanks for the report. This should be fixed by 5753ee2 and #154289 would avoid the situation in the first place.

Thanks!