[PATCH] D20162: [MSP430] PR27500 CodeGen: Rework branch-select pass

[Anton Korobeynikov via llvm-commits]

asl added inline comments.

================
Comment at: lib/Target/MSP430/MSP430BranchSelector.cpp:31
@@ -29,1 +30,3 @@
 
+static const int WordSize = 2;
+static const int DisplacementBits = 10;
----------------
pftbest wrote:
> asl wrote:
> > Why these are ints? And not inside namespace { ?
> `WordSize` is an int because it is involved in a calculations with negative numbers and that prevents nasty signed/unsigned promotion bugs. There is no reason for `DisplacementBits` to be signed. Both of them are used only inside `isInRange` function, so I can either move them there or move them inside a namespace and keep as global constants.
please move inside isInRange.

================
Comment at: lib/Target/MSP430/MSP430BranchSelector.cpp:49
@@ -41,1 +48,3 @@
+  /// the beginning of each basic block.
+  SmallVector<int, 16> BlockOffsets;
 
----------------
pftbest wrote:
> asl wrote:
> > Why this should be a member variable? It's effectively only used inside "expandBranches". Make it local there and you won't need all that error-prone BlockOffsets.clear() dance.
> I can make it local inside `runOnMachineFunction` and then pass it as a reference to `measureFunction` and `expandBranches`
This might also work, yes.


http://reviews.llvm.org/D20162