[Openmp-commits] [PATCH] D146277: Handle the unexpected inputs for pass HardwareLoops

Tue Mar 28 05:30:13 PDT 2023

samparker added inline comments.

================
Comment at: llvm/lib/CodeGen/HardwareLoops.cpp:330

+  assert(
+      Opts.Bitwidth.has_value() && Opts.Decrement.has_value() &&
----------------
XinWang10 wrote:
> samparker wrote:
> > samparker wrote:
> > > XinWang10 wrote:
> > > > fhahn wrote:
> > > > > It shouldn't be possible to trigger an assert when passing arguments to the pass. If there's an invalid combination, it should be handled gracefully, e.g. bailing out without doing anything (together with a debug message perhaps) or using a sane default value if it makes sense.
> > > > I'm not familiar with this pass so I'm not sure how to correctly handle the unexpected input, maybe someone can refine it later? But for now, the wrong input is unacceptable b/c it will lead to crash.
> > > So, `hardware-loop-counter-bitwidth` defaults to 32. so how about we set the default value of `HardwareLoopInfo.CountType` to `Int32Ty`?
> > Sorry, I didn't mean setting it here. `struct HardwareLoopInfo` is declared in `llvm/include/llvm/Analysis/TargetTransformInfo.h`, and I think giving the member `CountType` a default value should work.
> > Sorry, I didn't mean setting it here. `struct HardwareLoopInfo` is declared in `llvm/include/llvm/Analysis/TargetTransformInfo.h`, and I think giving the member `CountType` a default value should work.
> 
> Yes, I adjust the code. What I want to point out is that there could be 3 crash situation here, 1 in line 337 in raw file, one in isHardwareLoopCandidate which also use CountType, another in InsertLoopDec which use LoopDecrement.
Please see D147042 as an illustration of what I meant. Would it solve these problems?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146277