[PATCH] Beginning of alloca implementation for Mips fast-isel

Eric Christopher echristo at gmail.com
Wed Feb 18 22:02:26 PST 2015


Replies inline.


================
Comment at: lib/Target/Mips/MipsFastISel.cpp:1
@@ -1,2 +1,2 @@
 //===-- MipsastISel.cpp - Mips FastISel implementation
 //---------------------===//
----------------
rkotler wrote:
> echristo wrote:
> > Could you fix this comment (in a separate commit) while you're at it?
> Okay. I've copied the header for AArch64, making appropriate changes for Mips.
As I mentioned, please do this in a separate commit. It's also preapproved so just commit. Thanks.

================
Comment at: lib/Target/Mips/MipsFastISel.cpp:667
@@ +666,3 @@
+  if (Addr.isRegBase()) {
+    if (!isInt<16>(Addr.getOffset()))
+      Addr = materializeOffset(Addr);
----------------
rkotler wrote:
> echristo wrote:
> > I think the frequent offset checks are going to be inherently buggy. How about doing something ala ARMComputeAddress (or the equivalent in aarch64)?
> I will investigate this idea for a future patch but I don't want to do more here. Good point though. This kind of code occurs all over llvm so it's not out of style. I have 10 patches in the queue for mips fast-isel and I need to get caught up.
> 
> Which code were you thinking of in ARMFastIsel?
We fix this kind of code as we see it and it's in review right now (also, since you're in here and handling all of this right now). The name of the function is ARMSimplifyAddress and it's immediately after ARMComputeAddress.

================
Comment at: test/CodeGen/Mips/Fast-ISel/overflt.ll:6
@@ +5,3 @@
+
+ at x = common global [128000 x float] zeroinitializer, align 4
+ at y = global float* getelementptr inbounds ([128000 x float]* @x, i32 0, i32 0), align 4
----------------
rkotler wrote:
> echristo wrote:
> > Is this from C code?
> Originally. Almost every make check test I have written was derived from a standalone executable test. Many, but not all, were originally C code. This one seems to be.
> 
> I make sure the executable test works and then usually lop off "main" and compile with -emit-llvm and turn it into an LLVM test.
> 
> 
> 
> 
It works well then if you put the C code that you used in the file so that it can just be regenerated if something changes as a comment.

http://reviews.llvm.org/D6426

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






More information about the llvm-commits mailing list