[PATCH] Scalar/PHI code genration

Tobias Grosser tobias at grosser.es
Tue Feb 10 23:32:22 PST 2015


Hi Johannes,

a last minor comment.

Also, to make it clear. I think the general structure of the patch is great. There are some parts where I try to give some comments to better understand the patch or to improve readability, but none is affecting the general structure of the patch.

Looking forward to an updated version,
Tobias


================
Comment at: lib/CodeGen/BlockGenerators.cpp:321
@@ +320,3 @@
+      InstMapped = InstCopy;
+    }
+  }
----------------
jdoerfert wrote:
> grosser wrote:
> > I do not understand why this condition (and the forwarding of the instructions in copyInstruction) is necessary. If I remove the full if (InstCopy) block, all test cases still pass for me. Is the BBMap not already updated when the statements are copied?
> > 
> > For example:
> > 
> > ```
> >  // Compute NewStore before its insertion in BBMap to make the insertion
> >  // deterministic.
> > ​ BBMap[Store] = NewStore;
> > ​ return NewStore;
> > ```
> During the development this looked different and when I changed it I assumed that if there is a inst copy there is a mapping, however for synthezisable instructions there is none (currently), the same holds for phi nodes (but then there is no inst copy either).
I understand partially. ;-)
Are you saying this is needed for PHI nodes and synthezisable instructions, but we just don't have a test case? Or this was needed at some point, but is not needed any more?

http://reviews.llvm.org/D7513

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the llvm-commits mailing list