In D68328#1758448, @Joe wrote:

In D68328#1758323, @igorb wrote:

LGTM,
Thanks

Brilliant. Thank you.
I don't have commit access. Can you commit this on my behalf please? Or I can try and request commit access.

LGTM,
Thanks

adding @sanjoy for more thoughts on such a proposed SCEV change

In D68328#1743867, @Joe wrote:

Updated diff:

I am quite new to SCEV, but it really looks like it just shouldn't work on pointers sometimes. I've fixed it such that it works with custom dl on all tests, so hopefully this is okay.

Also fixed a few more places in which idx and ptr size are assumed to be the same.

Added tests which touch as many places I could manage where I changed ptr type to idx type. Many of the failures were caused due to the ptrtoint.

I think you also missed

@@ -9261,7 +9261,7 @@ unsigned SelectionDAG::InferPtrAlignment(SDValue Ptr) const {
   const GlobalValue *GV;
   int64_t GVOffset = 0;
   if (TLI->isGAPlusOffset(Ptr.getNode(), GV, GVOffset)) {
-    unsigned IdxWidth = getDataLayout().getIndexTypeSizeInBits(GV->getType());
+    unsigned IdxWidth = getDataLayout().getPointerTypeSizeInBits(GV->getType());
     KnownBits Known(IdxWidth);
     llvm::computeKnownBits(GV, Known, getDataLayout());

Surely this should stay as IndexType?

Hi Joseph,
Thank you very much for looking into this

Hi Joseph,
Sorry for the delay. I am currently checking your changes, I need a few more days to complete the investigation.

LGTM

LGTM

The change looks ok.
Could you please add mir test for InstructionSelector pass, similar to select-memop-scalar.mir.
Thanks

The x86 changes looks ok. Few minor comments.
Thanks.

LGTM,
Thanks

In D51197#1215499, @aemerson wrote:

x86 is still using UADDE, and it's generated only by IRTranslator, so if you do this then x86 will lose support for compiling llvm.uadd.with.overflow intrinsics. @igorb what do you think?

Which is exposing a hole in our general GISel testing, where individual changes in passes can result in overall support for some input IR to be lost.

LGTM

LGTM

LGTM

LGTM

Could you please consider to add ll tests also.

merged.

LGTM

LGTM

LGTM

Hi,
Could you please add more context in this patch also (svn diff --diff-cmd=diff -x -U999999)
Thanks

Hi,
Could you please add more context (svn diff --diff-cmd=diff -x -U999999)
Thanks

LGTM,
Thanks.

Hi,
Could you please add test for Legalizar pass?
I also suggest to add ll test to test all the passes together.

LGTM,
Thanks.

Could you please add test for Legalizar pass?

Hi Aditya ,
Unfortunately, I don't have time to work on this task

LGTM

LGTM

LGTM

LGTM

LGTM

Fixed CodeGen/X86/GlobalISel/phi.ll failing.

• rebase

fix according to comments.
32bit PIC mode not supported yet.

LGTM

Failed to build as a shared library -DBUILD_SHARED_LIBS=1

Hi Aditya,
I implemented missing functionality in https://reviews.llvm.org/rL312903
Could you please rebase.

• fix according to comments

• fix according to comments and rebase

X86 backend part - https://reviews.llvm.org/D37678
Could you please merge into you patch.
Regards.

• fix copy - paste

LGTM

LGTM

• fix according to comments

LGTM

LGTM

• rebase

• rebase

• rebase.

LGTM ,
Could you please make sure that tests cover the masked version of the instructions with DQI.

ping

• Fix according to comments and rebase.

ping

ping

LGTM

• Fix according to comments and rebase.

• add tests. Thanks.

In D34978#798826, @kristof.beyls wrote:

Hi Igor,

Would it be possible to add a regression test for this?
I'm wondering when this fails today. I guess it must be when the target has specified an action for e.g. TypeIdx 0, but not TypeIdx 1?
I'm just trying to understand if this should be an assert (if a target hasn't specified something fully enough) or an if statement.

Thanks,

Kristof

• Fix according to comments
• rebase ToT

• reabse
• Thanks for the review!

ping