[PATCH] D89021: BPF: fix incorrect DAG2DAG load optimization

[Yonghong Song via Phabricator via llvm-commits]

yonghong-song created this revision.
yonghong-song added reviewers: ast, anakryiko.
Herald added subscribers: llvm-commits, jfb, hiraditya.
Herald added a project: LLVM.
yonghong-song requested review of this revision.

Currently, bpf backend Instruction section DAG2DAG phase has 
an optimization to replace loading constant struct memeber
or array element with direct values. The reason is that these
locally defined struct or array variables may have their
initial values stored in a readonly section and early bpf 
ecosystem is not able to handle such cases.

Bpf ecosystem now can not only handle readonly sections,
but also global variables. global variable can also have
initialized data and global variable may or may not be constant,
i.e., global variable data can be put in .data section or .rodata
section. This exposed a bug in DAG2DAG Load optimization
as it did not check whether the global variable is constant
or not.

This patch fixed the bug by checking whether global variable,
representing the initial data, is constant or not and will not 
do optimization if it is not a constant.

Another bug is also fixed in this patch to check whether
the load is simple (not volatile/atomic) or not. If it is
not simple, we will not do optimization. To summary for 
globals:

- struct t var = { ... } ;  // no load optimization
- const struct t var = { ... }; // load optimization is possible
- volatile const struct t var = { ... }; // no load optimization


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D89021

Files:
  llvm/lib/Target/BPF/BPFISelDAGToDAG.cpp
  llvm/test/CodeGen/BPF/rodata_6.ll
  llvm/test/CodeGen/BPF/rodata_7.ll


Index: llvm/test/CodeGen/BPF/rodata_7.ll
===================================================================
--- /dev/null
+++ llvm/test/CodeGen/BPF/rodata_7.ll
@@ -0,0 +1,25 @@
+; RUN: llc -march=bpf < %s | FileCheck %s
+;
+; Source code:
+;   struct t1 { int a; };
+;   volatile const struct t1 data = { .a = 3 };
+;   int foo(void) {
+;     return data.a + 20;
+;   }
+; Compilation flag:
+;   clang -target bpf -O2 -S -emit-llvm test.c
+
+%struct.t1 = type { i32 }
+
+ at data = dso_local constant %struct.t1 { i32 3 }, align 4
+
+; Function Attrs: nofree norecurse nounwind
+define dso_local i32 @foo() local_unnamed_addr {
+entry:
+  %0 = load volatile i32, i32* getelementptr inbounds (%struct.t1, %struct.t1* @data, i64 0, i32 0), align 4
+  %add = add nsw i32 %0, 20
+; CHECK:   [[REG1:r[0-9]+]] = data ll
+; CHECK:   r0 = *(u32 *)([[REG1]] + 0)
+; CHECK:   r0 += 20
+  ret i32 %add
+}
Index: llvm/test/CodeGen/BPF/rodata_6.ll
===================================================================
--- /dev/null
+++ llvm/test/CodeGen/BPF/rodata_6.ll
@@ -0,0 +1,25 @@
+; RUN: llc -march=bpf < %s | FileCheck %s
+;
+; Source code:
+;   struct t1 { int a; };
+;   struct t1 data = { .a = 3 };
+;   int foo(void) {
+;     return data.a + 20;
+;   }
+; Compilation flag:
+;   clang -target bpf -O2 -S -emit-llvm test.c
+
+%struct.t1 = type { i32 }
+
+ at data = dso_local local_unnamed_addr global %struct.t1 { i32 3 }, align 4
+
+; Function Attrs: norecurse nounwind readonly
+define dso_local i32 @foo() local_unnamed_addr {
+entry:
+  %0 = load i32, i32* getelementptr inbounds (%struct.t1, %struct.t1* @data, i64 0, i32 0), align 4
+  %add = add nsw i32 %0, 20
+; CHECK:   [[REG1:r[0-9]+]] = data ll
+; CHECK:   r0 = *(u32 *)([[REG1]] + 0)
+; CHECK:   r0 += 20
+  ret i32 %add
+}
Index: llvm/lib/Target/BPF/BPFISelDAGToDAG.cpp
===================================================================
--- llvm/lib/Target/BPF/BPFISelDAGToDAG.cpp
+++ llvm/lib/Target/BPF/BPFISelDAGToDAG.cpp
@@ -254,7 +254,7 @@
   const LoadSDNode *LD = cast<LoadSDNode>(Node);
   uint64_t size = LD->getMemOperand()->getSize();
 
-  if (!size || size > 8 || (size & (size - 1)))
+  if (!size || size > 8 || (size & (size - 1)) || !LD->isSimple())
     return;
 
   SDNode *LDAddrNode = LD->getOperand(1).getNode();
@@ -342,7 +342,7 @@
                                             unsigned char *ByteSeq) {
   const GlobalVariable *V = dyn_cast<GlobalVariable>(Node->getGlobal());
 
-  if (!V || !V->hasInitializer())
+  if (!V || !V->hasInitializer() || !V->isConstant())
     return false;
 
   const Constant *Init = V->getInitializer();


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D89021.296843.patch
Type: text/x-patch
Size: 2615 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20201008/91929a09/attachment.bin>