[GlobalISel] gen-global-isel failed to work

Hi Leslie,

There should be a definition of GPR8RegClassID in $build_dir/lib/Target/AVR/AVRGenRegisterInfo.inc which should be included by AVRRegisterInfo.h. AArch64 includes its AArch64RegisterInfo.h in AArch64InstructionSelector.cpp but it seems that ARM gets it indirectly when it includes ARMSubtarget.h. It looks like you need to add '#include "AVRRegisterInfo.h"' to AVRInstructionSelector.h.

Hope that helps

Hi Daniel,

Thanks for your response!

脭脷 2017脛锚12脭脗19脠脮 18:53, Daniel Sanders 脨麓碌脌:

Hi Leslie,

There should be a definition of GPR8RegClassID in $build_dir/lib/Target/AVR/AVRGenRegisterInfo.inc which should be included by AVRRegisterInfo.h. AArch64 includes its AArch64RegisterInfo.h in AArch64InstructionSelector.cpp but it seems that ARM gets it indirectly when it includes ARMSubtarget.h. It looks like you need to add '#include "AVRRegisterInfo.h"' to AVRInstructionSelector.h.

Hope that helps

Still failed to work :frowning: https://github.com/xiangzhai/llvm/commit/1fc76db7f6fda156d2d0a2bafa6d8ea4c43a7e40
It must be my stupid bug... I need to read the Resources more carefully! http://llvm.org/docs/GlobalISel.html

Sorry, I've given you the wrong header. The one I gave you does include AVRGenRegisterInfo.inc but doesn't define the enum because it uses the wrong macros for that. The correct one is '#include "MCTargetDesc/AVRMCTargetDesc.h"'. You'll also need to 'include "AVRRegisterBanks.td"' somewhere in your .td files. At the moment, your register banks definitions aren't being used.

Hi Daniel,

Thanks for your hint! no errors :slight_smile: https://github.com/xiangzhai/llvm/commit/8cd3e548f70243e1e324d8395aeb5e27a14bd53c

I only implemented the skeleton of GlobalISel, removed ARM related code further, it needs to override several functions to make AVR NEW instruction selector to really work.

Sorry, I am apprentice of lowRISC, and meet new bug when porting GlobalISel to RISCV target https://github.com/xiangzhai/llvm/commit/b3f91ea54d9fee0ef7e73a32c6b8456bbe252811

In file included from /data/project/xiangzhai/llvm/lib/Target/RISCV/RISCVInstructionSelector.cpp:97:
/data/project/xiangzhai/llvm/build/lib/Target/RISCV/RISCVGenGlobalISel.inc:100:7: error: use of
undeclared identifier 'Subtarget'
if (Subtarget->is64Bit())
^

Even if no errors after comment `if (Subtarget->is64Bit())` in RISCVInstrInfo.td, but it is monkey patch, and it can't handle riscv64 without the if condition check.

And I noticed there is `if (Subtarget->useMovt(*MF))` in ARMInstrInfo.td too https://github.com/llvm-mirror/llvm/blob/master/lib/Target/ARM/ARMInstrInfo.td#L694

How ARM handle this? and I removed all ARM related code in RISCVXXX.cpp, only keep skeleton for easy debug, please give me some hint, thanks a lot!

Hi Leslie,

Sorry, I am apprentice of lowRISC, and meet new bug when porting GlobalISel
to RISCV target
https://github.com/xiangzhai/llvm/commit/b3f91ea54d9fee0ef7e73a32c6b8456bbe252811

In file included from
/data/project/xiangzhai/llvm/lib/Target/RISCV/RISCVInstructionSelector.cpp:97:
/data/project/xiangzhai/llvm/build/lib/Target/RISCV/RISCVGenGlobalISel.inc:100:7:
error: use of
      undeclared identifier 'Subtarget'
  if (Subtarget->is64Bit())
      ^

Even if no errors after comment `if (Subtarget->is64Bit())` in
RISCVInstrInfo.td, but it is monkey patch, and it can't handle riscv64
without the if condition check.

And I noticed there is `if (Subtarget->useMovt(*MF))` in ARMInstrInfo.td too
https://github.com/llvm-mirror/llvm/blob/master/lib/Target/ARM/ARMInstrInfo.td#L694

I haven't looked into this, but I would imagine that none of the
patterns using that are supported yet, and that's why we don't see any
errors for ARM yet.

How ARM handle this? and I removed all ARM related code in RISCVXXX.cpp,
only keep skeleton for easy debug, please give me some hint, thanks a lot!

ARMInstructionSelector has an ARMSubtarget reference called STI. We'll
probably have to rename that to Subtarget to support something like
this in the future.

What you need to do when you run into this kind of error is to add the
relevant members to your InstructionSelector. Each target may make
slightly different assumptions about what exists in the
InstructionSelector. This one is pretty obvious from the name, but in
the general case you might have to look at what exists in DAGISel.

Hope that helps,
Diana

Hi Leslie,

Sorry, I am apprentice of lowRISC, and meet new bug when porting GlobalISel
to RISCV target
https://github.com/xiangzhai/llvm/commit/b3f91ea54d9fee0ef7e73a32c6b8456bbe252811

In file included from
/data/project/xiangzhai/llvm/lib/Target/RISCV/RISCVInstructionSelector.cpp:97:
/data/project/xiangzhai/llvm/build/lib/Target/RISCV/RISCVGenGlobalISel.inc:100:7:
error: use of
     undeclared identifier 'Subtarget'
if (Subtarget->is64Bit())
     ^

Even if no errors after comment `if (Subtarget->is64Bit())` in
RISCVInstrInfo.td, but it is monkey patch, and it can't handle riscv64
without the if condition check.

And I noticed there is `if (Subtarget->useMovt(*MF))` in ARMInstrInfo.td too
https://github.com/llvm-mirror/llvm/blob/master/lib/Target/ARM/ARMInstrInfo.td#L694

I haven't looked into this, but I would imagine that none of the
patterns using that are supported yet, and that's why we don't see any
errors for ARM yet.

How ARM handle this? and I removed all ARM related code in RISCVXXX.cpp,
only keep skeleton for easy debug, please give me some hint, thanks a lot!

ARMInstructionSelector has an ARMSubtarget reference called STI. We'll
probably have to rename that to Subtarget to support something like
this in the future.

What you need to do when you run into this kind of error is to add the
relevant members to your InstructionSelector. Each target may make
slightly different assumptions about what exists in the
InstructionSelector. This one is pretty obvious from the name, but in
the general case you might have to look at what exists in DAGISel.

Unfortunately, adding the members to InstructionSelector isn't going to work in this case. The ImmLeaf predicates are currently static functions and only have access to the immediate being tested. We're going to have to change how these predicates are generated to give them access to the members of the InstructionSelector.

Hi Leslie. You're going to run into a challenge with GlobalISel and
the RISC-V backend due to our use of variable-sized register classes
(http://lists.llvm.org/pipermail/llvm-dev/2016-September/105027.html).
I believe that neither GlobalISel or FastISel have been updated to
support this concept.

Best,

Alex

Hi Diana,

Thanks for your kind response! you are my teacher, you answered my questions patiently and carefully in the mailing list, and I read your Slide https://archive.fosdem.org/2017/schedule/event/globalisel/attachments/slides/1657/export/events/attachments/globalisel/slides/1657/2017_FOSDEM_GlobalISel.pdf it is helpful for porting GlobalISel :slight_smile:

Hi Leslie,

Sorry, I am apprentice of lowRISC, and meet new bug when porting GlobalISel
to RISCV target
https://github.com/xiangzhai/llvm/commit/b3f91ea54d9fee0ef7e73a32c6b8456bbe252811

In file included from
/data/project/xiangzhai/llvm/lib/Target/RISCV/RISCVInstructionSelector.cpp:97:
/data/project/xiangzhai/llvm/build/lib/Target/RISCV/RISCVGenGlobalISel.inc:100:7:
error: use of
       undeclared identifier 'Subtarget'
   if (Subtarget->is64Bit())
       ^

Even if no errors after comment `if (Subtarget->is64Bit())` in
RISCVInstrInfo.td, but it is monkey patch, and it can't handle riscv64
without the if condition check.

And I noticed there is `if (Subtarget->useMovt(*MF))` in ARMInstrInfo.td too
https://github.com/llvm-mirror/llvm/blob/master/lib/Target/ARM/ARMInstrInfo.td#L694

I haven't looked into this, but I would imagine that none of the
patterns using that are supported yet, and that's why we don't see any
errors for ARM yet.

How ARM handle this? and I removed all ARM related code in RISCVXXX.cpp,
only keep skeleton for easy debug, please give me some hint, thanks a lot!

ARMInstructionSelector has an ARMSubtarget reference called STI. We'll
probably have to rename that to Subtarget to support something like
this in the future.

What you need to do when you run into this kind of error is to add the
relevant members to your InstructionSelector. Each target may make
slightly different assumptions about what exists in the
InstructionSelector. This one is pretty obvious from the name, but in
the general case you might have to look at what exists in DAGISel.

I will try.

Hi Leslie,

Sorry, I am apprentice of lowRISC, and meet new bug when porting GlobalISel
to RISCV target
https://github.com/xiangzhai/llvm/commit/b3f91ea54d9fee0ef7e73a32c6b8456bbe252811

In file included from
/data/project/xiangzhai/llvm/lib/Target/RISCV/RISCVInstructionSelector.cpp:97:
/data/project/xiangzhai/llvm/build/lib/Target/RISCV/RISCVGenGlobalISel.inc:100:7:
error: use of
      undeclared identifier 'Subtarget'
  if (Subtarget->is64Bit())
      ^

Even if no errors after comment `if (Subtarget->is64Bit())` in
RISCVInstrInfo.td, but it is monkey patch, and it can't handle riscv64
without the if condition check.

And I noticed there is `if (Subtarget->useMovt(*MF))` in ARMInstrInfo.td too
https://github.com/llvm-mirror/llvm/blob/master/lib/Target/ARM/ARMInstrInfo.td#L694

I haven't looked into this, but I would imagine that none of the
patterns using that are supported yet, and that's why we don't see any
errors for ARM yet.

How ARM handle this? and I removed all ARM related code in RISCVXXX.cpp,
only keep skeleton for easy debug, please give me some hint, thanks a lot!

ARMInstructionSelector has an ARMSubtarget reference called STI. We'll
probably have to rename that to Subtarget to support something like
this in the future.

What you need to do when you run into this kind of error is to add the
relevant members to your InstructionSelector. Each target may make
slightly different assumptions about what exists in the
InstructionSelector. This one is pretty obvious from the name, but in
the general case you might have to look at what exists in DAGISel.

Unfortunately, adding the members to InstructionSelector isn't going to work in this case. The ImmLeaf predicates are currently static functions and only have access to the immediate being tested. We're going to have to change how these predicates are generated to give them access to the members of the InstructionSelector.

Cool! I will try to hack lib/CodeGen/GlobalISel for learning, thanks!

Sorry, I am apprentice of lowRISC, and meet new bug when porting GlobalISel
to RISCV target
https://github.com/xiangzhai/llvm/commit/b3f91ea54d9fee0ef7e73a32c6b8456bbe252811

Hi Leslie. You're going to run into a challenge with GlobalISel and
the RISC-V backend due to our use of variable-sized register classes
(http://lists.llvm.org/pipermail/llvm-dev/2016-September/105027.html).
I believe that neither GlobalISel or FastISel have been updated to
support this concept.

I am reading the thread you mentioned in the mailing list, and I will try Krzysztof's patch, so it is an opportunity to hack lib/CodeGen just like RegAlloc :slight_smile: http://lists.llvm.org/pipermail/llvm-dev/2017-December/119741.html

I read your Slide
https://archive.fosdem.org/2017/schedule/event/globalisel/attachments/slides/1657/export/events/attachments/globalisel/slides/1657/2017_FOSDEM_GlobalISel.pdf
it is helpful for porting GlobalISel :slight_smile:

You might want to also watch the tutorial from this year's US LLVM
[1], since it's a bit more recent.

Cheers,
Diana

[1] https://www.youtube.com/watch?v=Zh4R40ZyJ2k

Hi Leslie,

Sorry, I am apprentice of lowRISC, and meet new bug when porting GlobalISel
to RISCV target
https://github.com/xiangzhai/llvm/commit/b3f91ea54d9fee0ef7e73a32c6b8456bbe252811

In file included from
/data/project/xiangzhai/llvm/lib/Target/RISCV/RISCVInstructionSelector.cpp:97:
/data/project/xiangzhai/llvm/build/lib/Target/RISCV/RISCVGenGlobalISel.inc:100:7:
error: use of
undeclared identifier ‘Subtarget’
if (Subtarget->is64Bit())
^

Even if no errors after comment if (Subtarget->is64Bit()) in
RISCVInstrInfo.td, but it is monkey patch, and it can’t handle riscv64
without the if condition check.

And I noticed there is if (Subtarget->useMovt(*MF)) in ARMInstrInfo.td too
https://github.com/llvm-mirror/llvm/blob/master/lib/Target/ARM/ARMInstrInfo.td#L694

I haven’t looked into this, but I would imagine that none of the
patterns using that are supported yet, and that’s why we don’t see any
errors for ARM yet.

How ARM handle this? and I removed all ARM related code in RISCVXXX.cpp,
only keep skeleton for easy debug, please give me some hint, thanks a lot!

ARMInstructionSelector has an ARMSubtarget reference called STI. We’ll
probably have to rename that to Subtarget to support something like
this in the future.

What you need to do when you run into this kind of error is to add the
relevant members to your InstructionSelector. Each target may make
slightly different assumptions about what exists in the
InstructionSelector. This one is pretty obvious from the name, but in
the general case you might have to look at what exists in DAGISel.

Unfortunately, adding the members to InstructionSelector isn’t going to work in this case. The ImmLeaf predicates are currently static functions and only have access to the immediate being tested. We’re going to have to change how these predicates are generated to give them access to the members of the InstructionSelector.

I’ve fixed this limitation in r321176 by making the predicate functions members of the InstructionSelector. Starting from that commit, you can resolve the problem with referencing
Subtarget by adding it to the RISCVInstructionSelector as Diana suggested.

Hi Leslie,

Sorry, I am apprentice of lowRISC, and meet new bug when porting GlobalISel
to RISCV target
https://github.com/xiangzhai/llvm/commit/b3f91ea54d9fee0ef7e73a32c6b8456bbe252811

In file included from
/data/project/xiangzhai/llvm/lib/Target/RISCV/RISCVInstructionSelector.cpp:97:
/data/project/xiangzhai/llvm/build/lib/Target/RISCV/RISCVGenGlobalISel.inc:100:7:
error: use of
undeclared identifier 'Subtarget'
if (Subtarget->is64Bit())
^

Even if no errors after comment `if (Subtarget->is64Bit())` in
RISCVInstrInfo.td, but it is monkey patch, and it can't handle riscv64
without the if condition check.

And I noticed there is `if (Subtarget->useMovt(*MF))` in ARMInstrInfo.td too
https://github.com/llvm-mirror/llvm/blob/master/lib/Target/ARM/ARMInstrInfo.td#L694

I haven't looked into this, but I would imagine that none of the
patterns using that are supported yet, and that's why we don't see any
errors for ARM yet.

How ARM handle this? and I removed all ARM related code in RISCVXXX.cpp,
only keep skeleton for easy debug, please give me some hint, thanks a lot!

ARMInstructionSelector has an ARMSubtarget reference called STI. We'll
probably have to rename that to Subtarget to support something like
this in the future.

What you need to do when you run into this kind of error is to add the
relevant members to your InstructionSelector. Each target may make
slightly different assumptions about what exists in the
InstructionSelector. This one is pretty obvious from the name, but in
the general case you might have to look at what exists in DAGISel.

Unfortunately, adding the members to InstructionSelector isn't going to work in this case. The ImmLeaf predicates are currently static functions and only have access to the immediate being tested. We're going to have to change how these predicates are generated to give them access to the members of the InstructionSelector.

I've fixed this limitation in r321176 by making the predicate functions members of the <Target>InstructionSelector. Starting from that commit, you can resolve the problem with referencing
Subtarget by adding it to the RISCVInstructionSelector as Diana suggested.

Thanks for your great work!

Sorry for my stupid... but after renamed `STI` to `Subtarget` https://github.com/xiangzhai/llvm/commit/819cd634157644146ce91a834bdc23e9301d93b1

it looked for `STI` again?

[ 48%] Building CXX object lib/Target/RISCV/CMakeFiles/LLVMRISCVCodeGen.dir/RISCVInstructionSelector.cpp.o
In file included from /data/project/xiangzhai/llvm/lib/Target/RISCV/RISCVInstructionSelector.cpp:97:
/data/project/xiangzhai/llvm/build/lib/Target/RISCV/RISCVGenGlobalISel.inc:191:65: error: use of
undeclared identifier 'STI'
AvailableFunctionFeatures = computeAvailableFunctionFeatures(&STI, &MF);
^

Please give me some hint, thanks a lot!

TableGen expects there to be a member called STI so you can't rename it but you could add Subtarget and initialize it to STI to get things working quickly. Later on, you can try to remove Subtarget again by changing the ImmLeaf predicates that need it.

Alternatively, I suppose we could have TableGen provide Subtarget as an alias. It would be a fairly small and useful change to make if it turns out we have a lot of these predicates.

Works, thanks for your hint :slight_smile: https://github.com/xiangzhai/llvm/commit/6d084db681b98b9f6e17d63e3e21f85b095119aa