[PATCH] D91050: [NFC] Add the EmitTargetCodeForConstantPool hook for target to customize it with MachineConstantPoolValue

[Qing Shan Zhang via Phabricator via llvm-commits]

steven.zhang added a comment.

In D91050#2472648 <https://reviews.llvm.org/D91050#2472648>, @nemanjai wrote:

> Hmm... I think it is very surprising if `DAG.getConstantPool()` does not return `ConstantPoolSDNode`. I am not in favour of any design that violates such seemingly reasonable assumptions.

But it has no harm or even better to remove such assumptions. Is it right ?

The motivation for this patch is for D91053 <https://reviews.llvm.org/D91053> (I should have documented it more clear). We indeed have the case that DAG.getConstantPool() doesn't return 'ConstantPoolSDNode'. This is what we want to do in PowerPC:

                                          ConstantPoolSDNode
                                          +----------------+
  ConstantFP         ConstantPool         |                |
  +-------+          +----------+         |                |
  |  1.5  +--------->+          +--------->xxxxxxxxxxxxxxxx|
  +-------+          +----------+         |                |
                                          |                |
                                          |                |
  ConstantFP         ConstantPool         |                |
  +-------+          +----------+         |                |
  |  3.0  +--------->+          +--------->xxxxxxxxxxxxxxxx|
  +-------+          +----------+         |                |
                                          +----------------+

When we are creating the constant pool from DAG, we want to return the same ConstantPoolSDNode but with different offset. This is done by ConstantPoolSDNode + ADD. And as we have the MachineConstantPoolValue to allow target to customize its constant pool, we'd better remove such kind of assumption. And it also make the code a bit clean and extensible, IMO. What do you think ?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D91050/new/

https://reviews.llvm.org/D91050