[PATCH] D48026: [ScopHelper] Provide support for recognising collective invariant loads

[Siddharth via Phabricator via llvm-commits]

bollu added inline comments.


================
Comment at: test/ScopDetect/collective_invariant_loads.ll:28
+%chpl_listNode_BaseDom_chpl_object = type { %chpl_object_object, %chpl_BaseDom_object*, %chpl_listNode_BaseDom_chpl_object* }
+%_distribution_DefaultDist = type { i64, %chpl_DefaultDist_object*, i8 }
+%_array_DefaultRectangularArr_2_int64_t_F__real64_int64_t = type { i64, %chpl_DefaultRectangularArr_2_int64_t_F__real64_int64_t_object*, i8 }
----------------
We can get rid of many of these type definitions, please remove the unnecesasary ones.


================
Comment at: test/ScopDetect/collective_invariant_loads.ll:47
+  %.01 = phi i64 [ 1, %2 ], [ %15, %3 ]
+  %4 = getelementptr inbounds %_array_DefaultRectangularArr_2_int64_t_F__real64_int64_t, %_array_DefaultRectangularArr_2_int64_t_F__real64_int64_t* %0, i64 0, i32 1
+  %5 = load %chpl_DefaultRectangularArr_2_int64_t_F__real64_int64_t_object*, %chpl_DefaultRectangularArr_2_int64_t_F__real64_int64_t_object** %4, align 8, !tbaa !2
----------------
We can probably rename `_array_DefaultRectangularArr_2_int64_t_F__real64_int64_t` to something shorter like `array_ty`


================
Comment at: test/ScopDetect/collective_invariant_loads.ll:51
+  %7 = load i64, i64* %6, align 8, !tbaa !14
+  %8 = mul nsw i64 %7, %.0
+  %9 = add nsw i64 %8, %.01
----------------
Can we give the variables sane names? If it's too much work, don't bother


================
Comment at: test/ScopDetect/collective_invariant_loads.ll:81
+
+!0 = !{i32 1, !"wchar_size", i32 4}
+!1 = !{!"clang version 7.0.0 (https://git.llvm.org/git/clang.git 02bcd70eea3567cd0577d4ed76031087c1a48ac3) (https://git.llvm.org/git/llvm.git e27401501e0d6f407c45ab7bc4c1b1e1c54f0356)"}
----------------
Please remove un-necessary metadata


https://reviews.llvm.org/D48026