[PATCH] D144879: [ConstraintElimination] Add function arguments to constraint system before solving

[Florian Hahn via Phabricator via llvm-commits]

fhahn added inline comments.


================
Comment at: llvm/include/llvm/Analysis/ConstraintSystem.h:16
 #include "llvm/ADT/SmallVector.h"
+#include "llvm/Support/Casting.h"
 
----------------
Not needed?


================
Comment at: llvm/include/llvm/Analysis/ConstraintSystem.h:23
 class Value;
-
+class Argument;
 class ConstraintSystem {
----------------
Is `Argument` used?Maybe `FunctionArgs` should use `Argument *` instead of `Value *`


================
Comment at: llvm/include/llvm/Analysis/ConstraintSystem.h:24
+class Argument;
 class ConstraintSystem {
   struct Entry {
----------------
keep newline here.


================
Comment at: llvm/include/llvm/Analysis/ConstraintSystem.h:72
   ConstraintSystem() {}
+  ConstraintSystem(SmallVector<Value *> FunctionArgs) {
+    NumVariables += FunctionArgs.size();
----------------
This passes the full vector by value. Use `ArrayRef<Value *>` instead.


================
Comment at: llvm/include/llvm/Analysis/ConstraintSystem.h:80
+      : Value2Index(Value2Index) {
+    NumVariables = Value2Index.size();
+  }
----------------
Initialise this like `Value2Index`, as in : Value2Index(Value2Index), NumVariables(Value2Index.size())`.


================
Comment at: llvm/lib/Transforms/Scalar/ConstraintElimination.cpp:143
+  ConstraintInfo(const DataLayout &DL, SmallVector<Value *> FunctionArgs)
+      : DL(DL) {
+    UnsignedCS = ConstraintSystem(FunctionArgs);
----------------
Initialise `UnsignedCS` and `SignedCS` like `DL`, as in `: UnsignedCS(FunctionArgs), SignedCS(FunctionArgs), DL(DL)` 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D144879/new/

https://reviews.llvm.org/D144879