[PATCH] Avoid illegal integer promotion in X86FastISel

Eric Christopher echristo at gmail.com
Mon Nov 11 17:19:43 PST 2013


Hi Duncan,

Few comments:

+  /// \brief Check if \c Add is an add that can be safely folded into \c GEP

Might want to elaborate with the actual conditions in the comment.
Also sentences end in a period.

+bool FastISel::canFoldAddIntoGEP(const User *GEP, const Value *Add)
+{

Style nit.

+  if (TD.getTypeSizeInBits(GEP->getType()) !=
+      TD.getTypeSizeInBits(Add->getType()))

+  if (isa<Instruction>(Add) &&
+      FuncInfo.MBBMap[cast<Instruction>(Add)->getParent()] != FuncInfo.MBB)

The indenting doesn't look quite right here.

Meta: clang-format would have fixed all of these for sure. :)

+; RUN: llc %s -O0 -o - | FileCheck %s
+
+target triple = "armv7-apple-ios"

Go ahead and fold the triple into RUN line for all of these.

2013-11-08-fastisel-gep-promote-before-add.ll

We don't generally use the date on testcases anymore.

-eric

On Fri, Nov 8, 2013 at 4:47 PM, Duncan Exon Smith <dexonsmith at apple.com> wrote:
> On Nov 7, 2013, at 2:21 PM, Eric Christopher <echristo at gmail.com> wrote:
>
>> Seems reasonable, pretty sure the ARM port has the same bug though so could you fix it in both places? Maybe pull it out into more general code somewhere?
>>
>> -eric
>
> Good call.
>
> Please see the updated patch attached below, which addresses X86, ARM and PPC bugs (and uses a pipe for FileCheck).
>



More information about the llvm-commits mailing list