[PATCH] Fix FastISel bug in boolean returns for PowerPC

[Hal Finkel]

Hi Samuel,

Thanks for working on this!

+      switch (VA.getLocInfo()) {
+        default:
+          llvm_unreachable("Unknown loc info!");
+          break;
+        case CCValAssign::Full:
+        case CCValAssign::AExt:
+        case CCValAssign::ZExt:
+          SrcReg = PPCMaterializeInt(C, MVT::i64, false);
+          break;
+        case CCValAssign::SExt:
+          SrcReg = PPCMaterializeInt(C, MVT::i64, true);
+          break;
+      }

Seems like this could just be:

  SrcReg = PPCMaterializeInt(C, MVT::i64,
                             VA.getLocInfo() == CCValAssign::SExt);

+define zeroext i1 @rettrue() nounwind uwtable ssp {
+entry:
+; ELF64: rettrue
+; ELF64: li 3, 1
+; ELF64: blr
+  ret i1 true
+}

Each of these tests should use a CHECK-LABEL marker when matching the test function name for better error recovery in the reporter should something fail. Like this:

+define zeroext i1 @rettrue() nounwind uwtable ssp {
+entry:
+; ELF64-LABEL: @rettrue
+; ELF64: li 3, 1
+; ELF64: blr
+  ret i1 true
+}

Otherwise, LGTM. Do you have commit access now?

 -Hal

----- Original Message -----
> From: "Samuel F Antao" <sfantao at us.ibm.com>
> To: llvm-commits at cs.uiuc.edu
> Sent: Wednesday, September 10, 2014 6:32:19 PM
> Subject: [PATCH] Fix FastISel bug in boolean returns for PowerPC
> 
> 
> 
> 
> Hi all,
> 
> For PPC targets, FastISel does not take the sign extension
> information into account when selecting return instructions whose
> operands are constants. A consequence of this is that the return of
> boolean values is not correct. E.g. the following program would
> (erroneously) return 0:
> 
> bool T(){
> return true;
> }
> 
> bool F(){
> return !T();
> }
> 
> inr main(){
> if(F())
> return 0;
> else
> return 1;
> }
> 
> The reason the output of this program is wrong is that ‘true’ sign is
> sign to 64-bit (0xfff..fff) and the negation only flips the least
> significant bit. This code pattern is used in some Clang’s unit
> tests, and this is the reason they are failing for PowerPC when
> building clang with the latest clang.
> 
> This patches fixes the problem by evaluating the sign extension
> information also for constants, forwarding this information to
> PPCMaterializeInt which takes this information to drive the sign
> extension during the materialization.
> 
> 
> The modified files are the following:
> 
> lib/Target/PowerPC/PPCFastISel.cpp
> test/CodeGen/PowerPC/fast-isel-ret.ll
> 
> Thanks,
> 
> Sam
> 
> (See attached file: llvm_fsel_ret.patch)
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> 

-- 
Hal Finkel
Assistant Computational Scientist
Leadership Computing Facility
Argonne National Laboratory