[PATCH] D19410: [scan-build] fix warnings emitted on LLVM ARM code base

Weiming Zhao via llvm-commits llvm-commits at lists.llvm.org
Fri May 6 14:04:23 PDT 2016


weimingz added inline comments.

================
Comment at: lib/Target/ARM/ARMFrameLowering.cpp:1610
@@ -1609,4 +1609,3 @@
   //        worth the effort and added fragility?
-  bool BigStack = (RS && (MFI->estimateStackSize(MF) +
-                              ((hasFP(MF) && AFI->hasStackFrame()) ? 4 : 0) >=
-                          estimateRSStackSizeLimit(MF, this))) ||
+  bool BigStack = ((MFI->estimateStackSize(MF) +
+                    ((hasFP(MF) && AFI->hasStackFrame()) ? 4 : 0) >=
----------------
weimingz wrote:
> rengolin wrote:
> > weimingz wrote:
> > > compnerd wrote:
> > > > rengolin wrote:
> > > > > Better ask @weimingz about this one.
> > > > Ugh, this is *subtle*.  If we are going to simplify this, please add a couple of asserts.
> > > > 
> > > >     assert(requiresRegisterScavenging() && "ARM requires register scavenging");
> > > >     assert(RS && "Target requiring register scavenging not provided register scavenger");
> > > Agree. We need to simplify the BigStack test. It's hard to read.
> > > 
> > >  It would be better if we refactor it to:
> > > 
> > > unsigned EstimatedStackSize  = MFI->estimateStackSize(MF) + ...
> > > 
> > > bool BgsTack = EstimatedStackSize >= estimatedRSStackSizeLImit() || ... || ...
> > >  
> > Weiming, can you do this one once you re-commit your patch?
> Sure. I will do.
Please review http://reviews.llvm.org/D19896


http://reviews.llvm.org/D19410





More information about the llvm-commits mailing list