[PATCH] D27853: [CodeGenPrep]Restructure promoting Ext to form ExtLoad

[Jun Bum Lim via Phabricator via llvm-commits]

junbuml added inline comments.


================
Comment at: lib/CodeGen/CodeGenPrepare.cpp:4305
     TotalCreatedInstsCost -= ExtCost;
+    if (TotalCreatedInstsCost < 0)
+      TotalCreatedInstsCost = 0;
----------------
junbuml wrote:
> mcrosier wrote:
> > You might also write this as
> > 
> >   TotalCreatedInstsCost = std::max(0, TotalCreatedInstsCost - ExtCost);
> > 
> > Can you please add a comment as to why we ensure TotalCreatedInstsCost is non-negative?  I believe it's because we're passing a signed value to tryToPromoteExts(), which expects an unsigned argument.
> > 
> > Lastly, is this something that could be fixed independently of this patch?  And do you have a test case that covers this change?
> Yes, the signed value TotalCreatedInstsCost is passed as unsigned argument,  the negative value of TotalCreatedInstsCost could mislead the cost model.   
> 
> Let me separate this as an independent patch with a test case. 
This is fixed in rL293307.


https://reviews.llvm.org/D27853