[PATCH] D42571: [MIPS] Don't crash on unsized extern types with -mgpopt

[Simon Dardis via Phabricator via llvm-commits]

sdardis added a comment.

I've seen this patch in the CHERI fork, thanks for upstreaming it. Some nits inlined and can you use update_llc_test_checks.py to generate the CHECK lines?

Thanks.



================
Comment at: lib/Target/Mips/MipsTargetObjectFile.cpp:140
+
+  // It is possible that the target type is unsized. In this case don't
+  // attempt to put it in the small data section. This happens e.g. when
----------------
Change this to: "It is possible that the type of the global is unsized, i.e. a declaration of a extern struct. In this case don't presume it is in the small data section. This happens e.g. when building the FreeBSD kernel."


================
Comment at: test/CodeGen/Mips/unsized-global.ll:2
+; Check that -mgpopt doesn't crash on unsized externals
+; RUN: llc -mtriple=mips64-unknown-freebsd -target-abi n64 -mgpopt -o - %s | FileCheck %s
+
----------------
Add -mattr=+noabicalls here.


================
Comment at: test/CodeGen/Mips/unsized-global.ll:4-7
+; ModuleID = 'uart_subr-fd65f8-reduce.test.c'
+source_filename = "uart_subr-fd65f8-reduce.test.c"
+target datalayout = "E-m:e-i8:8:32-i16:16:32-i64:64-n32:64-S128"
+target triple = "mips64-unknown-freebsd"
----------------
This metadata can be removed.


================
Comment at: test/CodeGen/Mips/unsized-global.ll:13
+
+; Function Attrs: norecurse nounwind readnone
+define i32 @d() local_unnamed_addr #0 {
----------------
This line can be removed.


================
Comment at: test/CodeGen/Mips/unsized-global.ll:14
+; Function Attrs: norecurse nounwind readnone
+define i32 @d() local_unnamed_addr #0 {
+entry:
----------------
You can drop the 'local_unnamed_addr #0" here.


================
Comment at: test/CodeGen/Mips/unsized-global.ll:14-32
+define i32 @d() local_unnamed_addr #0 {
+entry:
+  br label %for.cond
+
+for.cond:                                         ; preds = %for.cond, %entry
+  br i1 icmp ugt (%struct.a* @b, %struct.a* null), label %for.cond, label %for.end
+
----------------
sdardis wrote:
> You can drop the 'local_unnamed_addr #0" here.
This llvm-ir can all be simplified to:

   define %struct.a* @d() {
   entry:
     ret %struct.a* @b
   }


================
Comment at: test/CodeGen/Mips/unsized-global.ll:34-40
+attributes #0 = { norecurse nounwind readnone "correctly-rounded-divide-sqrt-fp-math"="false" "disable-tail-calls"="false" "less-precise-fpmad"="false" "no-frame-pointer-elim"="false" "no-infs-fp-math"="false" "no-jump-tables"="false" "no-nans-fp-math"="false" "no-signed-zeros-fp-math"="false" "no-trapping-math"="false" "stack-protector-buffer-size"="8" "target-cpu"="mips64" "target-features"="+mips64,+noabicalls" "unsafe-fp-math"="false" "use-soft-float"="false" }
+
+!llvm.module.flags = !{!0}
+!llvm.ident = !{!1}
+
+!0 = !{i32 1, !"wchar_size", i32 4}
+!1 = !{!"clang version 6.0.0 (https://github.com/llvm-mirror/clang.git 1131af2e23396b93b1004df1751bfa3fef7990be) (https://github.com/llvm-mirror/llvm.git a2788ab93c15f82a12423c907426f602a4fdc5a3)"}
----------------
All of this can be dropped as well.


Repository:
  rL LLVM

https://reviews.llvm.org/D42571