[PATCH] D26855: New unsafe-fp-math implementation for X86 target

[Alexey Bataev via Phabricator via llvm-commits]

ABataev added a comment.

Is this patch an NFC patch? If no, the test must be added



================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:14950
 SDValue DAGCombiner::BuildReciprocalEstimate(SDValue Op, SDNodeFlags *Flags) {
-  if (Level >= AfterLegalizeDAG)
-    return SDValue();
+  //  if (Level >= AfterLegalizeDAG)
+  return SDValue();
----------------
Why is this line commented? If it is not required (but I rather doubt) you should just remove it.


================
Comment at: lib/Target/X86/X86InstrInfo.cpp:9196-9198
+  for (MachineInstr::mop_iterator MOI = MI->operands_begin(),
+                                  MOE = MI->operands_end();
+       MOI != MOE; ++MOI) {
----------------
I believe this can be replaced by a range-based loop


================
Comment at: lib/Target/X86/X86InstrInfo.cpp:9203
+      // FIXME: should we deal with other types of operand like Immediate?
+      auto ConstantEntry = Constants[MO.getIndex()];
+      if (!ConstantEntry.isMachineConstantPoolEntry()) {
----------------
auto &?


================
Comment at: lib/Target/X86/X86InstrInfo.cpp:9219-9223
+  for (MachineFunction::iterator MBBI = MF.begin(), MBBE = MF.end();
+       MBBI != MBBE; ++MBBI) {
+    for (MachineBasicBlock::instr_iterator MII = MBBI->instr_begin(),
+                                           MIE = MBBI->instr_end();
+         MII != MIE; ++MII) {
----------------
Should these loops be range-based loops?


================
Comment at: lib/Target/X86/X86InstrInfo.cpp:9226-9228
+      for (MachineInstr::mop_iterator MOI = MI->operands_begin(),
+                                      MOE = MI->operands_end();
+           MOI != MOE; ++MOI) {
----------------
Range-based loop?


================
Comment at: lib/Target/X86/X86InstrInfo.cpp:9232-9234
+        if (MO.getReg() == DividentReg) {
+          return hasAllOnesOperand(MI);
+        }
----------------
No braces required


================
Comment at: lib/Target/X86/X86InstrInfo.cpp:9307-9308
+    // We need all ones value to be able to do (1 - C * X[i])
+    // load
+    // x86-32 PIC requires a PIC base register for constant pools.
+    unsigned PICBase = 0;
----------------
reformat this


https://reviews.llvm.org/D26855