[llvm-commits] [PATCH][Review Please] Optimize ARM Position Independent Code Generation

[Evan Cheng]

Hi Yin,

Comments:

+cl::opt<bool>
+OptimizeGOT("optimize-got", cl::Hidden,
+  cl::desc("Enable / disable ARM PIC generation using more compact formats"),
+  cl::init(false));
+

+extern cl::opt<bool> OptimizeGOT;
+

This is the wrong way to implement it even if it's temporary. Please have the option sets the value of a member variable instead.

In ARMBaseInstrInfo.cpp:

+  if (ACPV->isGlobalValue()) {
+    if (OptimizeGOT) {
+      const GlobalValue *GV = cast<ARMConstantPoolConstant>(ACPV)->getGV();
+      bool UseGOTOFF = GV->hasLocalLinkage() || GV->hasHiddenVisibility();
+      unsigned PCAdj =
+                MF.getTarget().getSubtarget<ARMSubtarget>().isThumb() ? 4 : 8;
+      if (UseGOTOFF) {
+          NewCPV = ARMConstantPoolConstant::Create(GV,
+                                       PCLabelId, ARMCP::CPValue, PCAdj);
+      } else {
+          NewCPV = ARMConstantPoolConstant::Create(GV,
+                                       PCLabelId, ARMCP::CPValue, PCAdj,
+                                                  ARMCP::GOTPREL, true);
+      }

Please add some comments on what's going on because it is not obvious. It seems to me that if duplicateCPV() needs to know that the option OptimizeGOT is set, then this is not designed correctly. That is, what information is missing from the instruction that passes after instruction selection must have implicit knowledge about?

Evan

On Aug 23, 2012, at 5:33 PM, Yin Ma wrote:

> Hi Evan,
>  
> This patch addressed the problem I submitted before.  The default
> Option has been set to off. A segment of redundant elimination code
> has been removed. I will commit the new redundant elimination later
> after it becomes available.
>  
> Thanks,
>  
>                              Yin
>  
>  
>  
> From: Evan Cheng [mailto:evan.cheng at apple.com] 
> Sent: Wednesday, August 01, 2012 11:12 AM
> To: Yin Ma
> Cc: llvm-commits at cs.uiuc.edu
> Subject: Re: [llvm-commits] [PATCH][Review Please] Optimize ARM Position Independent Code Generation
>  
> Thanks. This looks like a good change to ELF codegen. I am ok with the isel change.
>  
> However, I'm not ok with pre-ra load / store optimizer change. It's the wrong place to do it. MachineCSE should be eliminating the unnecessary PIC instructions. You might want to look at Darwin codegen and how it plays with MachineCSE.
>  
> Evan
>  
> On Aug 1, 2012, at 10:24 AM, Yin Ma <yinma at codeaurora.org> wrote:
> 
> 
> Hi Evan,
>  
> The attached pdf contains an example of this change. It can result
> In code size reduction for most of PIC enabled programs.
>  
> Thanks,
>  
>                               Yin
>  
> From: Evan Cheng [mailto:evan.cheng at apple.com] 
> Sent: Tuesday, July 31, 2012 5:48 PM
> To: Yin Ma
> Cc: llvm-commits at cs.uiuc.edu
> Subject: Re: [llvm-commits] [PATCH][Review Please] Optimize ARM Position Independent Code Generation
>  
> Hi Yin,
>  
> Can you provide some assembly code so we can understand what you are trying to do?
>  
> Thanks,
>  
> Evan
>  
> On Jul 30, 2012, at 10:55 AM, Yin Ma <yinma at codeaurora.org> wrote:
> 
> 
> 
> Hi
> 
> Currently LLVM ARM backend is still using the x86 way to generate position independent code. The algorithm retrieves the pointer of global offset table at first. However, GCC has been using the label relative offset to computer position independent code. The GCC way results smaller code size in ARM architecture. This patch is to implement GCC way to generate position independent code.
> 
> In the attachment.
> pic.diff is the code patch
> the rest of four files are the test validation results before and after code changed
> 
> Please give a review
>  
> Thanks,
>  
>                     Yin
> <report.simple.new.txt><report.simple.org.txt><cross.new><cross.org><pic.diff>_______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>  
> <pic.pdf>
>  
> <report.simple.new.txt><report.simple.org.txt><cross.new><cross.org><pic2.diff>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120903/3aef86e3/attachment.html>