Thanks for review comment. But this patch hide another corner case which the
%7 may be a proxyValue of another real value.
I am writing a new version to fix those corner case.

-----Original Message-----
From: [email protected]
[mailto:[email protected]] On Behalf Of Song, Ruiling
Sent: Wednesday, December 18, 2013 10:06 AM
To: Gong, Zhigang; [email protected]
Cc: Gong, Zhigang
Subject: Re: [Beignet] [PATCH 2/2] GBE: fix a corner case which refer to a
undefined value.

Really strange corner case, I thought that normally the basic blocks have
been sorted!
The patch looks good to me.

-----Original Message-----
From: [email protected]
[mailto:[email protected]] On Behalf Of Zhigang Gong
Sent: Tuesday, December 17, 2013 1:26 PM
To: [email protected]
Cc: Gong, Zhigang
Subject: [Beignet] [PATCH 2/2] GBE: fix a corner case which refer to a
undefined value.

Clang/llvm may generate some code similar to the following IRs:

... (there is no definition of %7)
  br label 2

label1:
  %10 = add  %7, %6
  ...
  ret

label2:
  %7 = ...
  br label1

The value %7 is assigned after label2 but is referred at label1.
>From the control flow, the IRs is valid. As the reference will be executed
after the assignment.

But our current virtual register allocation assume that all the values are
assigned before any usage from the instructions' order view which is
incorrect for this case.

This patch choose a simple way to fix this issue. It allocate register when
it fails to get one. And latter when emit the assignment instruction, it
just ignore the duplicate allocation.

Signed-off-by: Zhigang Gong <[email protected]>
---
 backend/src/llvm/llvm_gen_backend.cpp |    9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/backend/src/llvm/llvm_gen_backend.cpp
b/backend/src/llvm/llvm_gen_backend.cpp
index 5755ac4..2654eba 100644
--- a/backend/src/llvm/llvm_gen_backend.cpp
+++ b/backend/src/llvm/llvm_gen_backend.cpp
@@ -407,8 +407,10 @@ namespace gbe
      */
     ir::Register newScalar(Value *value, Value *key, Type *type, uint32_t
index) {
       const ir::RegisterFamily family = getFamily(ctx, type);
-      const ir::Register reg = ctx.reg(family);
       key = key == NULL ? value : key;
+      if (scalarMap.find(std::make_pair(key, index)) != scalarMap.end())
+        return scalarMap[std::make_pair(key, index)];
+      const ir::Register reg = ctx.reg(family);
       this->insertRegister(reg, key, index);
       return reg;
     }
@@ -971,8 +973,11 @@ namespace gbe
     if(isa<Constant>(value)) {
       Constant *c = dyn_cast<Constant>(value);
       return getConstantRegister(c, elemID);
-    } else
+    } else {
+      if (!(regTranslator.valueExists(value, elemID)))
+        this->newRegister(value);
       return regTranslator.getScalar(value, elemID);
+    }
   }
 
   INLINE Value *GenWriter::getPHICopy(Value *PHI) {
--
1.7.9.5

_______________________________________________
Beignet mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/beignet
_______________________________________________
Beignet mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/beignet

_______________________________________________
Beignet mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/beignet

Reply via email to