[PATCH] [AArch64] enable rematerialization of float 0 values

[Geoff Berry]

Attaching an updated patch since the diff formatting is messed up below.

--
Geoff Berry
Employee of Qualcomm Innovation Center, Inc.
 Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux
Foundation Collaborative Project

-----Original Message-----
From: llvm-commits-bounces at cs.uiuc.edu
[mailto:llvm-commits-bounces at cs.uiuc.edu] On Behalf Of Geoff Berry
Sent: Friday, March 20, 2015 1:02 PM
To: 'Tim Northover'; 'Matthias Braun'
Cc: 'LLVM Commits'
Subject: RE: [PATCH] [AArch64] enable rematerialization of float 0 values

Ignoring the larger picture for a second, what about the following change
instead?

diff --git a/lib/Target/AArch64/AArch64InstrInfo.td
b/lib/Target/AArch64/AArch64InstrInfo.td
index 57a23a0..baa6abf 100644
--- a/lib/Target/AArch64/AArch64InstrInfo.td
+++ b/lib/Target/AArch64/AArch64InstrInfo.td
@@ -2348,8 +2348,12 @@ defm UCVTF : IntegerToFP<1, "ucvtf", uint_to_fp>;
 
 defm FMOV : UnscaledConversion<"fmov">;
 
-def : Pat<(f32 (fpimm0)), (FMOVWSr WZR)>, Requires<[NoZCZ]>; -def :
Pat<(f64 (fpimm0)), (FMOVXDr XZR)>, Requires<[NoZCZ]>;
+// Add separate opcode for FMOV 0 so we can mark it as 
+isReMaterializable let isReMaterializable = 1, isCodeGenOnly = 1 in 
+defm FMOV0 : UnscaledConversion<"fmov">;
+
+def : Pat<(f32 (fpimm0)), (FMOV0WSr WZR)>, Requires<[NoZCZ]>; def : 
+Pat<(f64 (fpimm0)), (FMOV0XDr XZR)>, Requires<[NoZCZ]>;
 
 
//===----------------------------------------------------------------------=
==//
 // Floating point conversion instruction.

--
Geoff Berry
Employee of Qualcomm Innovation Center, Inc.
 Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux
Foundation Collaborative Project

-----Original Message-----
From: Tim Northover [mailto:t.p.northover at gmail.com]
Sent: Thursday, March 19, 2015 10:54 PM
To: Matthias Braun
Cc: Geoff Berry; LLVM Commits
Subject: Re: [PATCH] [AArch64] enable rematerialization of float 0 values

> There is an exception for constant physical registers
(MRI.isConstantPhysReg()) however which should cover wzr/xzr. So in this
case all should be fine.

I got there too, but also looked into isConstantPhysReg itself and couldn't
convince myself that it really knew what made a register constant (E.g. it
looked like it might classify sp as constant because it's reserved and has
no defs before FrameLowering).

> I am indeed wondering if the flag is that usefull, the default logic
appears to be reasonable safe to me. Except for the case of an instruction
that takes unreasonably long to compute, I don't see why you would not set
it on any instruction. Maybe I'll do some experiments of what happens when
you switch the default for that flag to true...

I was also worried by isReallyTriviallyReMaterializable's comments being
written in terms of performance assessment rather than correctness. Trouble
is, this kind of thing is rather difficult to test or even probe rigorously.

Cheers.

Tim.


_______________________________________________
llvm-commits mailing list
llvm-commits at cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
-------------- next part --------------
A non-text attachment was scrubbed...
Name: aarch64-remat-float0-take2.patch
Type: application/octet-stream
Size: 877 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150320/99521cd4/attachment.obj>