[patch] FastISel omitting unused parameters hurts debug info

[Evan Cheng]

   // Enter non-dead arguments into ValueMap for uses in non-entry BBs.
   for (Function::const_arg_iterator I = FuncInfo.Fn->arg_begin(),
          E = FuncInfo.Fn->arg_end(); I != E; ++I) {
-    if (!I->use_empty()) {
-      DenseMap<const Value *, unsigned>::iterator VI = LocalValueMap.find(I);
-      assert(VI != LocalValueMap.end() && "Missed an argument?");
+    DenseMap<const Value *, unsigned>::iterator VI = LocalValueMap.find(I);
+    if (VI != LocalValueMap.end())
       FuncInfo.ValueMap[I] = VI->second;
-    }

I think you need to update the comment. At least the part about "non-dead".

I'm neutral about this patch. I don't believe it will have material impact on compile time but I do wonder if there is another way to fix this.

Evan


On Jun 6, 2013, at 7:52 PM, David Blaikie <dblaikie at gmail.com> wrote:

> Hi Dan,
> 
> Here's the patch we talked about this afternoon - with the equivalent
> change made to the ARM side of fast ISel to match the X86 change I'd
> made. Turns out with the ARM change something does fail -
> CodeGen/ARM/fast-isel-call.ll. So it seems this change isn't neutral
> to codegen. (though I'm not sure how much we need to care deeply about
> the performance of unused parameters?) I assume the source of the
> change is the FIXME immediately proceeding the change to *FastISel.cpp
> - so perhaps this is an acceptable "we'll fix the underlying issue &
> this and some other things will get better".
> 
> I was wondering if you had any ideas on how this should be addressed
> (is this acceptable? in which case I should just update the test case
> (though I don't fully understand it yet). I've attached the two .s
> files, with my change (a.s) and without my change (b.s) (around
> r183465) along with the patch itself.
> 
> - David
> <a.s><b.s><missing_unused_parameter.diff>_______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits