[PATCH] D29896: [BypassSlowDivision] Refactor fast division insertion logic (NFC)

[Nikolai Bozhenov via Phabricator via llvm-commits]

n.bozhenov marked 3 inline comments as done.
n.bozhenov added inline comments.


================
Comment at: lib/Transforms/Utils/BypassSlowDivision.cpp:80
+  unsigned Opcode;
+  Instruction *SlowDiv;
+  IntegerType *SlowType;
----------------
jlebar wrote:
> This is actually SlowInstr or InstrToReplace or SlowDivOrRem -- we should be careful here and elsewhere not to call things divs that may be rems.
During this optimization we generally use 'slow division' to refer to both Div and Rem operations. I believe that strictly following your suggestion would be overkill. For example, the file itself and its main entry point are named BypassSlowDivision.cpp and llvm::bypassSlowDivision. I'm not sure if we should rename them into something like bypassSlowDivisionOrRemainder.

To make the naming somewhat less confusing I suggest using the full word (Division) to refer to both Div and Rem operations, and use the abbreviations (Div/Rem) to refer to specific operation kinds. According to that, SlowDiv should be renamed into SlowDivision, FastDivInsertionTask into FastDivisionInsertionTask, and isDivisionOp into isDivOp. What do you think?



https://reviews.llvm.org/D29896